Refactor/implementation smell

jraft-core/src/main/java/com/alipay/sofa/jraft/core/NodeImpl.java

@@ -1272,34 +1272,42 @@ private void stepDown(final long term, final boolean wakeupCandidate, final Stat
         if (!this.state.isActive()) {
             return;
         }
+
+        handleStateSpecificActions(status);
+        resetLeaderIdAndState(term, status);
+        handleReplicatorActions(wakeupCandidate);
+        handleTransferActions();
+        restartElectionTimer();
+    }
+
+    private void handleStateSpecificActions(final Status status) {
         if (this.state == State.STATE_CANDIDATE) {
             stopVoteTimer();
         } else if (this.state.compareTo(State.STATE_TRANSFERRING) <= 0) {
             stopStepDownTimer();
             this.ballotBox.clearPendingTasks();
-            // signal fsm leader stop immediately
             if (this.state == State.STATE_LEADER) {
                 onLeaderStop(status);
             }
         }
-        // reset leader_id
-        resetLeaderId(PeerId.emptyPeer(), status);
+    }
 
-        // soft state in memory
+    private void resetLeaderIdAndState(long term, Status status) {
+        resetLeaderId(PeerId.emptyPeer(), status);
         this.state = State.STATE_FOLLOWER;
         this.confCtx.reset();
         updateLastLeaderTimestamp(Utils.monotonicMs());
         if (this.snapshotExecutor != null) {
             this.snapshotExecutor.interruptDownloadingSnapshots(term);
         }
-
-        // meta state
         if (term > this.currTerm) {
             this.currTerm = term;
             this.votedId = PeerId.emptyPeer();
             this.metaStorage.setTermAndVotedFor(term, this.votedId);
         }
+    }
 
+    private void handleReplicatorActions(boolean wakeupCandidate) {
         if (wakeupCandidate) {
             this.wakingCandidate = this.replicatorGroup.stopAllAndFindTheNextCandidate(this.conf);
             if (this.wakingCandidate != null) {
@@ -1308,15 +1316,18 @@ private void stepDown(final long term, final boolean wakeupCandidate, final Stat
         } else {
             this.replicatorGroup.stopAll();
         }
+    }
+
+    private void handleTransferActions() {
         if (this.stopTransferArg != null) {
             if (this.transferTimer != null) {
                 this.transferTimer.cancel(true);
             }
-            // There is at most one StopTransferTimer at the same term, it's safe to
-            // mark stopTransferArg to NULL
             this.stopTransferArg = null;
         }
-        // Learner node will not trigger the election timer.
+    }
+
+    private void restartElectionTimer() {
         if (!isLearner()) {
             this.electionTimer.restart();
         } else {
@@ -1732,7 +1743,7 @@ private boolean isLeaderLeaseValid() {
         if (checkLeaderLease(monotonicNowMs)) {
             return true;
         }
-        checkDeadNodes0(this.conf.getConf().getPeers(), monotonicNowMs, false, null);
+        checkAndUpdateLeaderWithDeadNodes(this.conf.getConf().getPeers(), monotonicNowMs, false, null);
         return checkLeaderLease(monotonicNowMs);
     }
 
@@ -2213,7 +2224,7 @@ private boolean checkDeadNodes(final Configuration conf, final long monotonicNow
         // Ensure quorum nodes alive.
         final List<PeerId> peers = conf.listPeers();
         final Configuration deadNodes = new Configuration();
-        if (checkDeadNodes0(peers, monotonicNowMs, true, deadNodes)) {
+        if (checkAndUpdateLeaderWithDeadNodes(peers, monotonicNowMs, true, deadNodes)) {
             return true;
         }
         if (stepDownOnCheckFail) {
@@ -2227,8 +2238,8 @@ private boolean checkDeadNodes(final Configuration conf, final long monotonicNow
         return false;
     }
 
-    private boolean checkDeadNodes0(final List<PeerId> peers, final long monotonicNowMs, final boolean checkReplicator,
-                                    final Configuration deadNodes) {
+    private boolean checkAndUpdateLeaderWithDeadNodes(final List<PeerId> peers, final long monotonicNowMs,
+                                                      final boolean checkReplicator, final Configuration deadNodes) {
         final int leaderLeaseTimeoutMs = this.options.getLeaderLeaseTimeoutMs();
         int aliveCount = 0;
         long startLease = Long.MAX_VALUE;

jraft-rheakv/rheakv-core/src/main/java/com/alipay/sofa/jraft/rhea/client/failover/impl/FailoverClosureImpl.java

@@ -57,6 +57,23 @@ public FailoverClosureImpl(CompletableFuture<T> future, boolean retryOnInvalidEp
         this.retryRunner = retryRunner;
     }
 
+    private boolean hasRetriesLeft(int retriesLeft) {
+        return retriesLeft > 0;
+    }
+
+    private boolean isInvalidPeerError(Errors error) {
+        return ErrorsHelper.isInvalidPeer(error);
+    }
+
+    private boolean isInvalidEpochError(Errors error) {
+        return ErrorsHelper.isInvalidEpoch(error);
+    }
+
+    public boolean shouldRetryOnError(int retriesLeft, boolean retryOnInvalidEpoch, Errors error) {
+        return hasRetriesLeft(retriesLeft)
+               && (isInvalidPeerError(error) || (retryOnInvalidEpoch && isInvalidEpochError(error)));
+    }
+
     @SuppressWarnings("unchecked")
     @Override
     public void run(final Status status) {
@@ -66,8 +83,7 @@ public void run(final Status status) {
         }
 
         final Errors error = getError();
-        if (this.retriesLeft > 0
-            && (ErrorsHelper.isInvalidPeer(error) || (this.retryOnInvalidEpoch && ErrorsHelper.isInvalidEpoch(error)))) {
+        if (shouldRetryOnError(this.retriesLeft, this.retryOnInvalidEpoch, error)) {
             LOG.warn("[Failover] status: {}, error: {}, [{}] retries left.", status, error, this.retriesLeft);
             this.retryRunner.run(error);
         } else {

jraft-test/src/main/java/com/alipay/sofa/jraft/test/atomic/HashAlgorithm.java

@@ -71,8 +71,11 @@ public long hash(final String k) {
                 break;
             case KETAMA_HASH:
                 byte[] bKey = computeMd5(k);
-                rv = (long) (bKey[3] & 0xFF) << 24 | (long) (bKey[2] & 0xFF) << 16 | (long) (bKey[1] & 0xFF) << 8
-                     | bKey[0] & 0xFF;
+                long fourthByteShifted = (long) (bKey[3] & 0xFF) << 24;
+                long thirdByteShifted = (long) (bKey[2] & 0xFF) << 16;
+                long secondByteShifted = (long) (bKey[1] & 0xFF) << 8;
+                long firstByte = bKey[0] & 0xFF;
+                rv = fourthByteShifted | thirdByteShifted | secondByteShifted | firstByte;
                 break;
 
         }