Skip to content

HDDS-16218. Fix SCM HA log messages that drop values and stack traces - #11046

Open
rjgoyln wants to merge 1 commit into
apache:masterfrom
rjgoyln:HDDS-16218
Open

HDDS-16218. Fix SCM HA log messages that drop values and stack traces#11046
rjgoyln wants to merge 1 commit into
apache:masterfrom
rjgoyln:HDDS-16218

Conversation

@rjgoyln

@rjgoyln rjgoyln commented Aug 18, 2026

Copy link
Copy Markdown

What changes were proposed in this pull request?

Four log statements in the SCM HA code print something other than what they promise, and all of them are on failure paths. The checkpoint-install failure line advertises the checkpoint transaction index but shows the exception's text in its place, because SLF4J only treats the final argument as a throwable — so the index is dropped and that line carries no stack trace of its own. The revert path a few lines later never prints an index either: its message has one placeholder for two values, so the state SCM fell back to goes unrecorded before the process terminates. In addSCM and removeSCM the SCM id is concatenated onto the format string instead of being passed as an argument, which leaves the Ratis reply in the slot meant for the id:

Failed to add new SCM: <ratis reply>. Ratis reply: {}<scmId>

These paths run only once something has already gone wrong, which is when an operator needs the values they omit. Behaviour is unchanged; only the rendered messages differ.

What is the link to the Apache JIRA

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

How was this patch tested?

mvn -pl :hdds-server-scm test -Dtest=TestSCMHAManagerImpl passes (3 tests; the class exercises the addSCM/removeSCM paths), and checkstyle:check on the module reports no violations. The rendering was verified against slf4j-reload4j 2.0.18, the binding Ozone ships: with the exception ahead of the index, MessageFormatter extracts no throwable, the index appears nowhere and no trace is logged; with the order corrected, the index renders and the trace is attached. No test asserts any of these strings, so none is added for text-only changes.

Generated-by: Claude Code (Claude Opus 5)

SLF4J only promotes the last argument to a throwable, so the checkpoint
install failure logged the exception in the slot meant for the transaction
index and dropped the index itself. The revert path of the same method
never recorded the index of the state SCM fell back to before terminating,
and addSCM/removeSCM folded the SCM id into the format string, leaving the
Ratis reply where the id belongs. These paths only run when something has
already gone wrong, which is exactly when an operator needs the values.
Copilot AI lite review requested due to automatic review settings August 18, 2026 12:19

Copilot AI 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.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@rjgoyln
rjgoyln marked this pull request as ready for review August 18, 2026 15:14

@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.

Thanks for the patch! @ss77892 LGTM +1

@rjgoyln

rjgoyln commented Aug 19, 2026

Copy link
Copy Markdown
Author

Thanks for the patch! @ss77892 LGTM +1

Thank for the review!

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.

3 participants