Siyao Meng created HDDS-16735:
---------------------------------
Summary: A writer's own hsync makes its own conditional commit
permanently unsatisfiable
Key: HDDS-16735
URL: https://issues.apache.org/jira/browse/HDDS-16735
Project: Apache Ozone
Issue Type: Bug
Reporter: Siyao Meng
Attachments: MC-5-conditional-commit-survives-own-hsync.patch
h3. Mechanism
A writer opens a key with If-None-Match: *, so its open key freezes
{{expectedDataGeneration}} at the sentinel 0. Three code decisions then compose
badly.
First, the condition survives hsync: {{OMKeyCommitRequest.java:297-302}}
snapshots {{newOpenKeyInfo = omKeyInfo.copyObject()}} before the builder at
{{:328}} sets {{setExpectedDataGeneration(null)}}, so the open key record
written back at {{:401}} still carries the condition.
Second, hsync publishes anyway: {{:405}} calls {{getKeyTable().addCacheEntry}}
unconditionally, hsync included, stamping {{updateID = trxnLogIndex}}. The
writer's own hsync is what makes the key exist.
Third, the commit time recheck has no same writer exemption:
{{validateAtomicRewrite}} at {{:316}} compares the frozen condition against the
current record, and its If-None-Match branch at {{:630}} throws
{{ATOMIC_WRITE_CONFLICT}} whenever {{existing != null}} regardless of who put
it there. {{isSameHsyncKey}} is computed at {{:229-245}} but consumed only for
quota accounting at {{:344}} and never reaches the check.
h3. Trigger
{{ozone.fs.hsync.enabled}} and {{ozone.client.hbase.enhancements.allowed}},
both of which default to false, plus a client calling {{hsync()}} on a
{{rewriteKey}} or {{createKeyIfNotExists}} stream.
h3. Impact
The writer is told its conditional write failed while its own partial data is
live and readable under that key, and the key can never be closed. The liveness
run confirms this is permanent rather than transient: both OM repair paths,
lease recovery and the open key cleanup re-commit, fail the same check forever,
leaving a failing background request every interval. No automatic recovery
exists.
h3. Reproduction
PASS. Reproduced from an RPC client application shape with hsync enabled.
Artifacts under {{confirmation/MC-5/}} and {{repro/}}. The liveness
configuration {{MC_hunt_s1_liveness.cfg}} establishes that the unsatisfiable
state is terminal rather than a window.
h3. Suggested fix
Give the commit time recheck a same writer exemption: pass the already computed
{{isSameHsyncKey}} ({{:229-245}}) into {{validateAtomicRewrite}} and skip the
check when the existing record is this client's own hsync record. The same
exemption must cover {{isRecovery}}, which resolves to that same client id.
Alternatively clear {{expectedDataGeneration}} on the open key snapshot taken
for the hsync write back ({{:302}}), so the condition is consumed by the first
commit that satisfies it. A later overwrite by a different writer stays
detectable, since the hsync record carries {{HSYNC_CLIENT_ID}}.
Add coverage to {{TestOMKeyCommitRequest}}: conditional open plus hsync plus
final commit for both the sentinel and generation forms, the recovery and
cleanup service re-commit shapes, plus the negative case where a different
writer's key appears between hsync and commit, which must still conflict.
h3. Patch
[^MC-5-conditional-commit-survives-own-hsync.patch], against
20e6a1c0f6039248373efd3eb1d7f2afcf7f1535.
Takes the first option above. {{validateAtomicRewrite}} in
{{OMKeyCommitRequest}} gains an {{existingIsOwnHsyncRecord}} parameter, and
both call sites, {{OMKeyCommitRequest}} and {{OMKeyCommitRequestWithFSO}}, pass
the already computed {{isSameHsyncKey || isRecovery}}. When the record the
condition would be judged against is this writer's own hsync record, the
condition was already judged by the commit that published it, so it is skipped.
{{isRecovery}} is needed alongside {{isSameHsyncKey}} because the latter is
computed before the recovery branch reassigns {{writerClientId}}, so a lease
recovery of the writer's own hsync record does not match it.
Covered by two tests in the existing {{TestOMKeyCommitRequest}}, both inherited
by the FSO subclass. {{testAtomicCreateIfNotExistsSurvivesThisWritersOwnHsync}}
opens a create-if-absent key, hsyncs it, then commits, and asserts the commit
succeeds and the open key is gone. It fails with {{ATOMIC_WRITE_CONFLICT}}
without the change, which is this bug.
{{testAtomicCreateIfNotExistsStillConflictsWithAnotherWriterAfterHsync}} is the
guard against over exempting: another writer's key appearing between the hsync
and the commit must still conflict, and it passes both with and without the
change. Checkstyle clean. The 23 tests in each of those two classes pass.
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]
