Skip to content

HDDS-10628. Display if safemode exit was via force exit command.#6494

Merged
adoroszlai merged 4 commits intoapache:masterfrom
sadanand48:HDDS-10628
Apr 24, 2024
Merged

HDDS-10628. Display if safemode exit was via force exit command.#6494
adoroszlai merged 4 commits intoapache:masterfrom
sadanand48:HDDS-10628

Conversation

@sadanand48
Copy link
Contributor

What changes were proposed in this pull request?

Safe mode rules are not calculated/updated after a safemode force exit, If a user looks at the SCM web UI after a force exit , it would give an impression that SCM is in safemode by looking at the rules not being satisfied.
Displaying if safe mode was forcefully exited will be helpful in this case.

What is the link to the Apache JIRA

https://issues.apache.org/jira/browse/HDDS-10628

How was this patch tested?

Manually
Screenshot 2024-04-08 at 4 59 22 PM

@sadanand48 sadanand48 changed the title Hdds 10628 HDDS-10628. Display if safemode exit was via force exit command. Apr 8, 2024
Copy link
Contributor

@myskov myskov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @sadanand48 for the patch. Just a minor comment from me.

@kerneltime kerneltime requested a review from sodonnel April 8, 2024 16:21
@kerneltime
Copy link
Contributor

cc @Tejaskriya

Copy link
Contributor

@sumitagrawl sumitagrawl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comment on lines 326 to 328
public boolean isSafeModeExitForceful() {
return forceExitSafeMode.get();
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the name of the variable is forceExitSafeMode the setter is also setForceExitSafeMode can we keep this method name in line: isForceExitSafeMode()

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

Copy link
Contributor

@Tejaskriya Tejaskriya left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM +1

@adoroszlai adoroszlai requested review from ayushtkn and myskov April 23, 2024 16:30
Copy link
Member

@ayushtkn ayushtkn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@adoroszlai adoroszlai merged commit 95b2caa into apache:master Apr 24, 2024
@adoroszlai
Copy link
Contributor

Thanks @sadanand48 for the patch, @ayushtkn, @myskov, @sumitagrawl, @Tejaskriya for the review.

jojochuang pushed a commit to jojochuang/ozone that referenced this pull request May 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants