Siyao Meng created HDDS-16736:
---------------------------------
Summary: CommitKey is not idempotent and the S3 gRPC path cannot
use the Ratis retry cache, so a retry reports KEY_NOT_FOUND for a durable write
Key: HDDS-16736
URL: https://issues.apache.org/jira/browse/HDDS-16736
Project: Apache Ozone
Issue Type: Bug
Reporter: Siyao Meng
h3. Mechanism
The first successful commit deliberately destroys the evidence that it
happened: {{OMKeyCommitRequest.java:385-390}} tombstones the open key with the
comment "So that this key can't be committed again", and {{:257-262}} throws
{{KEY_NOT_FOUND}} when a re-executed commit cannot find it. {{CommitKey}} is
therefore non idempotent by construction, and the only thing hiding that is the
Ratis retry cache.
That dedup is unavailable on the S3 path.
{{OzoneManagerServiceGrpc.java:90-97}} builds a fresh {{Server.Call}} per
request with {{getClientId()}} returning {{UUIDUtil.randomUUIDBytes()}}
({{:120}}) and a per request {{callCount}} {{AtomicInteger}}, so every retry
presents a new {{(ClientId, callId)}} pair and can never hit the cache. The
code comment at {{:97}} names the random {{ClientId}} as intentional.
Cache expiry, OM restart and leader change are three further routes to the same
loss.
h3. Trigger
An ordinary RPC timeout retry on the gRPC OM transport. Nothing above re-issues
the whole PUT, since {{KeyOutputStream}} retries at block level without
re-issuing {{openKey}}.
h3. Impact
The write becomes durable while the client is told {{KEY_NOT_FOUND}}, surfacing
as 404 or 412 to an S3 caller and as a final stream close failure to a native
client. Under If-None-Match: * every contender, including the actual winner,
concludes it lost.
Bounded rather than destructive: the data is durable rather than corrupted, and
the externally visible harm needs the gRPC OM transport, because the default
Hadoop RPC retry cache masks it.
h3. Reproduction
PASS. Artifacts under {{confirmation/MC-4/}} and {{repro/}}, driven from the
counterexample {{spec/output/r3_MC_hunt_s2.out}} against invariant
{{DurableWriteIsAcknowledged}}.
h3. Suggested fix
Make {{CommitKey}} idempotent instead of relying on RPC layer dedup: when the
open key is absent, consult the key table for an entry whose {{updateID}} or
metadata identifies this writer's client id and the same content, and return OK
rather than {{KEY_NOT_FOUND}}.
Independently, derive a stable {{(ClientId, callId)}} for the S3 gateway gRPC
endpoint from the inbound {{OMRequest}} so {{checkRetryCache()}} can dedup
there too. That also closes the cache expiry, restart and leader change routes,
which the first change only partially covers.
h3. No patch attached
Unlike the other four findings in this batch, this one is filed as analysis
only, because both halves of the fix are decisions for reviewers rather than
mechanical changes.
Making {{CommitKey}} idempotent needs a writer identity to match on, and the
committed record deliberately does not keep one: the non hsync path strips
{{HSYNC_CLIENT_ID}} from the record before it is written, with the comment that
hsync metadata must not reach the final key. So "consult the key table for an
entry that identifies this writer" requires adding a persisted marker to the
committed record, a change to the on disk format.
Deriving a stable {{(ClientId, callId)}} for the gRPC endpoint has the same
problem one layer down. {{OMRequest}} carries a {{clientId}}, but it is a per
client instance value held in a final field of
{{OzoneManagerProtocolClientSideTranslatorPB}}, so it is the same for every
request that client makes, and {{OMRequest}} carries no call id at all. A
correct retry cache key on that path therefore needs a new protocol field, not
a different reading of the existing ones.
Both are small changes once the shape is agreed. Neither should be chosen by
the reporter.
Found by TLA+ model checking and code review of the OM conditional create,
overwrite and delete 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]
