-
Notifications
You must be signed in to change notification settings - Fork 9.2k
HDFS-17536. RBF: Format safe-mode related logic and fix a race #6844
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
💔 -1 overall
This message was automatically generated. |
| if (isCacheStale) { | ||
| if (!safeMode) { | ||
| enter(); | ||
| enter(false); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
|
Hi @ZanderXu Please check checkstyle and spotbugs report by Yetus before check it in. |
|
💔 -1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
|
We're closing this stale PR because it has been open for 100 days with no activity. This isn't a judgement on the merit of the PR in any way. It's just a way of keeping the PR queue manageable. |
Both
RouterAdminServer#enterSafeMode()andRouterSafemodeService#periodicInvoke()#leavecan 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 toRouterServiceState.SAFEMODERouterSafemodeService#periodicInvoke()#leavegot true when checkingsafeMode && !isSafeModeSetManuallyRouterAdminServer#enterSafeMode()setsafeModeandisSafeModeSetManuallytotrueRouterAdminServer#enterSafeMode()gettruewhen checking safe-modeRouterSafemodeService#periodicInvoke()#leavecallleave()to leave safe-mode.This RBF is not in safe-mode and
safeModeisfalse, butisSafeModeSetManuallyistrue.