[
https://issues.apache.org/jira/browse/HDDS-16743?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Siyao Meng updated HDDS-16743:
------------------------------
Description:
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.
* 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. Patch
[^MC-2-persist-retry-cache-replies.patch], against
20e6a1c0f6039248373efd3eb1d7f2afcf7f1535, is a proposed fix that takes the
second option above. It is offered for review, since it adds a table and
touches the write path.
A new {{retryCacheTable}} maps the transaction index to the client ID, the call
ID and the serialized {{OMResponse}}. {{OzoneManagerDoubleBuffer}} writes the
entry in the same batch as the transaction, so the reply and the write it
belongs to are flushed together or not at all. {{OzoneManagerStateMachine}}
loads the table at startup and on reinitialize, and {{checkRetryCache()}}
answers from it when the Ratis retry cache misses. Entries are dropped a block
at a time by index, which keeps between one and two times
{{ozone.om.ratis.server.retry.cache.persisted.count}} of them (default 10000, 0
disables). The default matches {{ozone.om.unflushed.transaction.max.count}},
because the calls at risk are the ones in flight when an OM stops.
Covered by {{testRetryCacheWithRestartedOMs}} in 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 fails with {{KEY_ALREADY_EXISTS}} without the
change. Without the wait for the flush it passes even unpatched, because the
OMs replay the rename on restart. {{testRetryCacheTable}} in the existing
{{TestOzoneManagerDoubleBuffer}} covers the write, the purge and a transaction
with no client call. The OM Ratis unit suites,
{{TestOzoneManagerHAWithAllRunning}}, {{TestOzoneManagerHASnapshot}},
{{TestOMRatisSnapshots}} and {{TestOzoneConfigurationFields}} pass. Checkstyle
clean.
Open points for reviewers:
* Replies that carry credentials are persisted like any other, for example S3
secrets, delegation tokens and STS credentials. The OM DB already holds S3
secrets, but this is a new place for them. Excluding those request types is
possible.
* The write path gains one put per transaction, including the serialized reply.
Its cost has not been measured.
* No layout feature is added, following {{lifecycleScanStateTable}}. Downgrade
has not been tested.
* The window is a number of transactions, not a time. A retry of a call older
than the window is still applied again, as today.
[^MC-2-retry-cache-restart-reproduction.patch] is the reproduction test alone,
for anyone who wants to see the failure without the fix.
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).
was:
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).
> 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
> Priority: Critical
> Attachments: MC-2-persist-retry-cache-replies.patch,
> 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.
> * 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. Patch
> [^MC-2-persist-retry-cache-replies.patch], against
> 20e6a1c0f6039248373efd3eb1d7f2afcf7f1535, is a proposed fix that takes the
> second option above. It is offered for review, since it adds a table and
> touches the write path.
> A new {{retryCacheTable}} maps the transaction index to the client ID, the
> call ID and the serialized {{OMResponse}}. {{OzoneManagerDoubleBuffer}}
> writes the entry in the same batch as the transaction, so the reply and the
> write it belongs to are flushed together or not at all.
> {{OzoneManagerStateMachine}} loads the table at startup and on reinitialize,
> and {{checkRetryCache()}} answers from it when the Ratis retry cache misses.
> Entries are dropped a block at a time by index, which keeps between one and
> two times {{ozone.om.ratis.server.retry.cache.persisted.count}} of them
> (default 10000, 0 disables). The default matches
> {{ozone.om.unflushed.transaction.max.count}}, because the calls at risk are
> the ones in flight when an OM stops.
> Covered by {{testRetryCacheWithRestartedOMs}} in 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 fails with {{KEY_ALREADY_EXISTS}}
> without the change. Without the wait for the flush it passes even unpatched,
> because the OMs replay the rename on restart. {{testRetryCacheTable}} in the
> existing {{TestOzoneManagerDoubleBuffer}} covers the write, the purge and a
> transaction with no client call. The OM Ratis unit suites,
> {{TestOzoneManagerHAWithAllRunning}}, {{TestOzoneManagerHASnapshot}},
> {{TestOMRatisSnapshots}} and {{TestOzoneConfigurationFields}} pass.
> Checkstyle clean.
> Open points for reviewers:
> * Replies that carry credentials are persisted like any other, for example S3
> secrets, delegation tokens and STS credentials. The OM DB already holds S3
> secrets, but this is a new place for them. Excluding those request types is
> possible.
> * The write path gains one put per transaction, including the serialized
> reply. Its cost has not been measured.
> * No layout feature is added, following {{lifecycleScanStateTable}}.
> Downgrade has not been tested.
> * The window is a number of transactions, not a time. A retry of a call older
> than the window is still applied again, as today.
> [^MC-2-retry-cache-restart-reproduction.patch] is the reproduction test
> alone, for anyone who wants to see the failure without the fix.
> 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]
