[ 
https://issues.apache.org/jira/browse/HDFS-17536?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17849849#comment-17849849
 ] 

ASF GitHub Bot commented on HDFS-17536:
---------------------------------------

goiri commented on code in PR #6844:
URL: https://github.com/apache/hadoop/pull/6844#discussion_r1616463020


##########
hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/router/RouterSafemodeService.java:
##########
@@ -174,7 +167,7 @@ public void periodicInvoke() {
     // Always update to indicate our cache was updated
     if (isCacheStale) {
       if (!safeMode) {
-        enter();
+        enter(false);

Review Comment:
   Wouldn't make more sense to use `leave()`?
   It would be good to have unified enter and leave, and just have a call to 
each.





> RBF: Format safe-mode related logic and fix a race 
> ---------------------------------------------------
>
>                 Key: HDFS-17536
>                 URL: https://issues.apache.org/jira/browse/HDFS-17536
>             Project: Hadoop HDFS
>          Issue Type: Improvement
>            Reporter: ZanderXu
>            Assignee: ZanderXu
>            Priority: Major
>              Labels: pull-request-available
>
> RBF: Format safe-mode related logic and fix a race.
>  
> Both {{RouterAdminServer#enterSafeMode()}} and 
> {{RouterSafemodeService#periodicInvoke()#leave}} can change the router state 
> at the same time.
> Safe-mode change logic should be condensed into one method. And some races 
> may happen in the current implementation, such as:
>  # {{RouterAdminServer#enterSafeMode()}} set router stat to 
> {{RouterServiceState.SAFEMODE}}
>  # {{RouterSafemodeService#periodicInvoke()#leave}} got true when checking 
> {{safeMode && !isSafeModeSetManually}}
>  # {{RouterAdminServer#enterSafeMode()}} set {{safeMode}} and 
> {{isSafeModeSetManually}} to {{true}}
>  # {{RouterAdminServer#enterSafeMode()}} get {{true}} when checking safe-mode
>  # {{RouterSafemodeService#periodicInvoke()#leave}} call {{leave()}} to leave 
> safe-mode.
> This RBF is not in safe-mode and {{safeMode}} is {{{}false{}}}, but 
> {{isSafeModeSetManually}} is {{{}true{}}}.



--
This message was sent by Atlassian Jira
(v8.20.10#820010)

---------------------------------------------------------------------
To unsubscribe, e-mail: hdfs-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: hdfs-issues-h...@hadoop.apache.org

Reply via email to