amaliujia commented on a change in pull request #1314:
URL: https://github.com/apache/hadoop-ozone/pull/1314#discussion_r485371150



##########
File path: 
hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/ha/MockSCMHAManager.java
##########
@@ -78,8 +79,8 @@ public void start() throws IOException {
    * {@inheritDoc}
    */
   @Override
-  public boolean isLeader() {
-    return isLeader;
+  public Optional<Long> isLeader() {

Review comment:
       another challenge is how to test such change. ideally we can have a 
minicluster setup with configurable nodes so we can simulate the scenario of 
split brian.  

##########
File path: 
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/statemachine/StateContext.java
##########
@@ -478,7 +551,26 @@ public void execute(ExecutorService service, long time, 
TimeUnit unit)
   public SCMCommand getNextCommand() {
     lock.lock();
     try {
-      return commandQueue.poll();
+      initTermOfLeaderSCM();
+      if (!termOfLeaderSCM.isPresent()) {
+        return null;      // not ready yet
+      }
+
+      while (true) {
+        SCMCommand<?> command = commandQueue.poll();
+        if (command == null) {
+          return null;
+        }
+
+        updateTermOfLeaderSCM(command);
+        if (command.getTerm() == termOfLeaderSCM.get()) {

Review comment:
       I am confused on when `termOfLeaderSCM` is updated to the newest leader 
term. Is `termOfLeaderSCM` updated in during leader selection? 




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[email protected]



---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to