[
https://issues.apache.org/jira/browse/HDDS-16735?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Siyao Meng updated HDDS-16735:
------------------------------
Attachment: MC-5-conditional-commit-survives-own-hsync.patch
> 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
> Priority: Critical
> 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]
