Siyao Meng created HDDS-16743:
---------------------------------

             Summary: OM retry cache does not survive a restart, so a client 
retry of an applied write is applied again and reports a false failure
                 Key: HDDS-16743
                 URL: https://issues.apache.org/jira/browse/HDDS-16743
             Project: Apache Ozone
          Issue Type: Bug
            Reporter: Siyao Meng
         Attachments: MC-2-retry-cache-restart-reproduction.patch

h3. Mechanism
What makes an OM write at most once is the Ratis retry cache record keyed by 
{{ClientInvocationId(clientId, callId)}}, and that record lives only in process 
memory. Ratis writes it when it applies the entry, nothing persists it, and 
nothing rebuilds it. The Raft log and the OM's applied index are durable, and 
after a clean restart the state machine is already at the end of the log, so 
Ratis replays nothing.

A restarted OM therefore still holds the committed entry proving the call ran, 
and has lost the only record that would stop it running again. 
{{checkRetryCache()}} ({{OzoneManagerRatisServer.java:546-548}}) misses, 
{{OMExecutionFlow}} has no second layer of dedup, and Ratis consults the same 
empty cache on submit.

h3. Trigger
A client write is committed and applied, and the reply is lost because the OM 
goes down (rolling restart, crash, pod replacement). The OM comes back with an 
empty retry cache, and the Hadoop RPC retry presents the original client ID and 
call ID.

This holds whenever the retry reaches an OM process that did not apply the 
entry itself: a single OM, a restart of every OM, or a restarted node that 
regains leadership. A plain failover to a peer that stayed up is covered, 
because every replica that applies the entry creates the record. That is the 
case {{TestOzoneManagerHAWithStoppedNodes#testRetryCacheWithDownedOM}} tests.

It also needs the entry to have been flushed to the OM DB before the OM went 
down. An OM that stops with the entry applied but not yet flushed replays it 
from the Raft log on restart, and the replay recreates the record. So a crash 
right after the write is partly self healing, while a restart a moment later is 
not.

h3. Impact
A second log entry with the same {{ClientInvocationId}} is committed and 
applied, and the client gets a different answer for the same call. For 
{{RenameKey}} the rename that durably succeeded is reported as 
{{KEY_ALREADY_EXISTS}}. Nothing here is specific to rename: any request type 
that is not idempotent on its own is exposed in the same way, and {{CommitKey}} 
is the case described in HDDS-16736.

h3. Reproduction
PASS. With the real client and a real OM restart, {{OzoneBucket.renameKey}} 
threw {{KEY_ALREADY_EXISTS}} for a rename that had happened, in 2 of 3 
iterations, with only a short delay before the restart as timing help. With the 
client ID and call ID pinned the way {{testRetryCacheWithDownedOM}} does it: 
one committed entry for the invocation and no retry cache record after the 
restart, two committed entries after the retry, and the retry answered 
{{KEY_ALREADY_EXISTS}}. The control, the same duplicate without a restart, is 
absorbed and answered OK. Artifacts under {{confirmation/MC-2/}} and 
{{repro/}}, driven from the counterexample 
{{spec/output/r3_MC_deep_s3restart_apply.out}} against invariant 
{{AtMostOnceApply}}.

h3. Relation to HDDS-16736
HDDS-16736 is about the gRPC transport never being able to use the retry cache, 
and names OM restart in passing as a further route to the same loss. This issue 
is that route on the default Hadoop RPC transport, for all request types, with 
its own reproduction.

h3. Suggested fix
Make the record as durable as the log it guards, or make the requests 
idempotent.

* Rebuild the retry cache on startup from the log entries of the last retry 
cache expiry window, without applying them again. This is a change in Ratis.
* Or persist the invocation ID and result for the recent window in the OM DB, 
in the same batch as the transaction, and have {{checkRetryCache()}} fall back 
to it on a miss. That also closes the expiry and leader change routes named in 
HDDS-16736.
* Or make each non idempotent request recognise its own earlier success, which 
needs a writer identity on the stored record, as discussed in HDDS-16736.

h3. No fix patch attached
All three options are design decisions for reviewers: a Ratis change, a new 
persisted table, or a per request change to the stored record. None is a 
mechanical change.

[^MC-2-retry-cache-restart-reproduction.patch] is a reproduction only, against 
20e6a1c0f6039248373efd3eb1d7f2afcf7f1535. It adds 
{{testRetryCacheWithRestartedOMs}} to the existing 
{{TestOzoneManagerHAWithStoppedNodes}}, next to the failover test it mirrors: 
the same rename, a wait for every OM to apply and flush it, a restart of every 
OM, then the same call again. It asserts the retry is answered OK, and fails at 
this commit with {{KEY_ALREADY_EXISTS}}. Without the wait for the flush the 
same test passes, because the OMs replay the rename on restart.

Found by TLA+ model checking and code review of the OM client retry cache and 
at most once mutation paths under HDDS-15926, on commit 
20e6a1c0f6039248373efd3eb1d7f2afcf7f1535. Checked against open HDDS issues for 
duplicates before filing, in addition to the repository's merged git history. 
Generated with Specula (Claude Opus 5).



--
This message was sent by Atlassian Jira
(v8.20.10#820010)

---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to