Skip to content

HDDS-11620. Log SCM finalization completion - #10963

Open
F64116045 wants to merge 2 commits into
apache:masterfrom
F64116045:HDDS-11620-scm-finalization-complete-log
Open

HDDS-11620. Log SCM finalization completion#10963
F64116045 wants to merge 2 commits into
apache:masterfrom
F64116045:HDDS-11620-scm-finalization-complete-log

Conversation

@F64116045

@F64116045 F64116045 commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

What changes were proposed in this pull request?

SCM logs when upgrade finalization crosses the FINALIZATION_STARTED and
MLV_EQUALS_SLV checkpoints, but it did not log after crossing
FINALIZATION_COMPLETE. This made it difficult for operators to determine
from the SCM log whether finalization completed successfully.

This change logs the FINALIZATION_COMPLETE checkpoint after the finalizing
marker is removed successfully. It also extends the SCM finalization test to
verify that the completion message is emitted when finalization advances to
the completed state, but not when SCM is already finalized.

What is the link to the Apache JIRA

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

How was this patch tested?

mvn -pl hadoop-hdds/server-scm -am -Dtest=TestScmFinalization \
  -Dsurefire.failIfNoSpecifiedTests=false -DskipShade -DskipRecon \
  -DskipDocs test
mvn -pl hadoop-hdds/server-scm -DskipTests -DskipShade -DskipRecon \
  -DskipDocs checkstyle:check

CI: https://github.com/F64116045/ozone/actions/runs/31136447694

@F64116045
F64116045 marked this pull request as ready for review August 7, 2026 03:56

@chihsuan chihsuan left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nice improvement! LGTM. 👍

Left one small, non-blocking suggestion.

// correct order.
StatusAndMessages status =
manager.finalizeUpgrade(UUID.randomUUID().toString());
LogCapturer logCapturer =

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

What do you think about adding a failure-path test for removeFinalizingMark()? Verifying that no completion log is emitted would make the intended behavior explicit.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Thanks @chihsuan for reviewing.

What do you think about adding a failure-path test for removeFinalizingMark()?

I think this makes sense. Added a test for this. Thanks for the suggestion!

@sreejasahithi sreejasahithi left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks @F64116045 for this improvement
LGTM

@chungen0126 chungen0126 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

+1 LGTM

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.

4 participants