HDDS-11470. Report OM snapshot installation failures to Ratis - #11086
HDDS-11470. Report OM snapshot installation failures to Ratis#11086andyhuangdev wants to merge 2 commits into
Conversation
chungen0126
left a comment
There was a problem hiding this comment.
Thanks @andyhuangdev for working on this.
| throw new CompletionException( | ||
| new IOException("Failed to install snapshot from OM leader " + leaderNodeId)); | ||
| } | ||
| return termIndex; |
There was a problem hiding this comment.
I don't think this is the correct approach. A null return value for termIndex shouldn't always result in an exception.
There was a problem hiding this comment.
Thanks for pointing this out. I agree that treating every null TermIndex as an exception is too broad, since null is also used for cases where snapshot installation is unavailable or should not proceed.
I propose moving the failure handling to OzoneManager.installSnapshotFromLeader(). A genuine installCheckpoint() failure would be propagated as an IOException, while the existing non-exceptional null cases would retain their current semantics. OzoneManagerStateMachine already converts an IOException into an exceptional future for Ratis.
I will also update the tests to verify that:
- a
nullresult still completes normally; - an
IOExceptioncompletes the future exceptionally; and - a checkpoint installation failure is propagated as an
IOException.
Does this approach align with what you had in mind?
https://issues.apache.org/jira/browse/HDDS-11470
When OM checkpoint installation fails, the snapshot-install callback previously
completed normally with a null result. Ratis could therefore treat the request
as completed without a successful snapshot installation.
Complete the callback exceptionally when OM reports a failed installation.
Add unit coverage for both successful and failed callback completion.
Test: