[ 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