[GitHub] [hbase] Apache9 commented on pull request #1826: HBASE-24438 Don't update TaskMonitor when deserializing ServerCrashProcedure

[GitBox <gi...@apache.org>]

Apache9 commented on pull request #1826:
URL: https://github.com/apache/hbase/pull/1826#issuecomment-637222180


   I think the problem here is in the implementation of updateProgress? We pass updateState as false when calling it in the deserialize method, but the only place to update the currentRunningState is in the updateProgress method, so when calling from deserialize the currentRunningState will always be null.
   
   And the updateProgress method is totally a mess. The order of the execution it really strange...
   I think it should be like this
   ```
       String msg = "Processing ServerCrashProcedure of " + serverName;
       if (status == null) {
         status = TaskMonitor.get().createStatus(msg);
       }
       if (updateState) {
         currentRunningState = getCurrentState();
       }
       if (currentRunningState == ServerCrashState.SERVER_CRASH_FINISH) {
         status.markComplete(msg + " done");
         return;
       }
       int childrenLatch = getChildrenLatch();
       status.setStatus(msg + " current State " + currentRunningState
           + (childrenLatch > 0 ? "; remaining num of running child procedures = " + childrenLatch
               : ""));
   ```
   
   And we should call updateProgress(true) in deserializeStateData. Could you please try if this can fix your problem?
   
   Thanks.


----------------------------------------------------------------
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:
users@infra.apache.org