[
https://issues.apache.org/jira/browse/HDDS-16850?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Siyao Meng updated HDDS-16850:
------------------------------
Attachment: TestBugMC5RenameRecreateAutoCommit.java
> After an OBS or LEGACY rename and recreate of an hsync'ed key, the hard lease
> auto commit empties the new key and deletes blocks still in use
> ---------------------------------------------------------------------------------------------------------------------------------------------
>
> Key: HDDS-16850
> URL: https://issues.apache.org/jira/browse/HDDS-16850
> Project: Apache Ozone
> Issue Type: Bug
> Reporter: Siyao Meng
> Priority: Critical
> Attachments:
> MC-5-reject-close-of-hsync-key-replaced-without-marker.patch,
> TestBugMC5RenameRecreateAutoCommit.java
>
>
> h3. Mechanism
> {{OmMetadataManagerImpl.getExpiredOpenKeys}} treats an open key as hsync'ed
> when its own {{HSYNC_CLIENT_ID}} matches its client id, then builds the auto
> commit from the committed key entry with the same name, without checking that
> this entry is the one that client hsync'ed.
> {{OMKeyCommitRequest.validateAndUpdateCache}} does not check it either: it
> rejects the commit only when the open key carries {{DELETED_HSYNC_KEY}} or
> {{OVERWRITTEN_HSYNC_KEY}}.
> {{OMKeyRenameRequest}} (OBJECT_STORE and LEGACY) has no open file check,
> unlike {{OMKeyRenameRequestWithFSO}} (HDDS-8545), and does not touch the
> writer's open key. A key later created at the old name finds no existing
> entry at commit time, so it sets no marker either. The auto commit then
> commits the abandoned open key with the new key's length and block list:
> {{OmKeyInfo.updateLocationInfoList}} drops the new key's block as unknown,
> the abandoned writer's own block is queued as uncommitted, and the new key is
> queued as an old version.
> The defect also applies to FILE_SYSTEM_OPTIMIZED buckets. There a rename of
> an open hsync'ed file is rejected, but the open key can still be left
> unmarked while another key takes the name, and the auto commit then pairs
> them the same way.
> h3. Trigger
> {{ozone.fs.hsync.enabled}} and {{ozone.hbase.enhancements.allowed}} are true
> (both default false), OBJECT_STORE or LEGACY bucket.
> # Writer 1 writes {{k1}} and hsyncs, then stops writing without closing.
> # {{k1}} is renamed to {{k2}}.
> # Writer 2 creates a new {{k1}} and closes it.
> # Writer 1's open key reaches {{ozone.om.lease.hard.limit}} (default 7d) and
> the next cleanup run auto commits it.
> h3. Impact
> * Writer 2's committed {{k1}} is replaced by an entry built from writer 1's
> open key, with writer 2's length and no blocks, and writer 2's block is
> queued in {{deletedTable}}.
> * The renamed {{k2}} still references writer 1's block, which is queued in
> {{deletedTable}} too, so {{KeyDeletingService}} sends it to SCM for deletion
> while {{k2}} points at it.
> * No client gets an error. Nothing restores the blocks.
> h3. Reproduction
> PASS, deterministic, unmodified source, for OBJECT_STORE and LEGACY buckets.
> [^TestBugMC5RenameRecreateAutoCommit.java] ({{hadoop-ozone/ozone-manager}})
> runs one real OM through {{OzoneManagerProtocol}} with a 200 ms hard lease
> limit, uses only public write requests ({{openKey}}, {{allocateBlock}},
> {{hsyncKey}}, {{renameKey}}, {{commitKey}}) and then calls the real
> {{OpenKeyCleanupTask}} of a second, never started {{OpenKeyCleanupService}}
> once. Output on the OBS bucket: before the cleanup "k1 length 500 blocks
> [13046326171], k2 length 1000 blocks [13046326021], deleted blocks []", after
> it "k1 length 500 blocks [], k2 length 1000 blocks [13046326021], deleted
> blocks [13046326171, 13046326021]". A passing test means the defect is
> present. Physical deletion on datanodes was not run.
> h3. Patch
> [^MC-5-reject-close-of-hsync-key-replaced-without-marker.patch], against
> a6b7bdb937109ba2893688b41a89470bc95c88cf. It also applies to master at
> b0aa6475b78.
> A commit that is not a recovery commit is rejected with {{KEY_NOT_FOUND}}
> when the open key carries the committing client's own {{HSYNC_CLIENT_ID}} and
> the committed entry at its name is not the key that client hsync'ed (or there
> is none). This holds for hsync commits as well as for the close, as for the
> existing {{DELETED_HSYNC_KEY}} and {{OVERWRITTEN_HSYNC_KEY}} markers, and
> uses the same result code. The first hsync of an open key is not affected,
> because the open key gets {{HSYNC_CLIENT_ID}} only in that commit. The check
> compares the client's own id, as {{getExpiredOpenKeys}} does, so an open key
> that carries another client's {{HSYNC_CLIENT_ID}} copied with the metadata of
> an open hsync'ed key (S3 {{CopyObject}}, {{ozone sh key cp}}) still commits.
> The auto commit can then no longer commit another key's data, and nothing is
> queued for deletion. A writer whose hsync'ed key was renamed away now gets
> {{KEY_NOT_FOUND}} on its next hsync or close, as it already does after a
> delete or an overwrite, so it can no longer put its blocks back at the old
> name while the renamed key still references them. An open key that was never
> hsync'ed can still overwrite the name. {{OMKeyCommitRequestWithFSO}} gets the
> same check. No request field is added.
> The check changes what the apply writes, so it is enabled by a new
> {{OzoneManagerVersion.HSYNC_COMMIT_VALIDATION}} (101) through
> {{OMVersionManager.isAllowed}}. Until the cluster is finalized to that
> version every OM applies the commit as before, so OMs on different software
> versions never apply the same log entry differently, and the defect remains
> until finalization. HDDS-16848 adds the same {{HSYNC_COMMIT_VALIDATION}}
> constant for a related commit check, so whichever lands second drops its
> copy; the number itself is a placeholder to be agreed at merge.
> Side effects: the abandoned open key is never cleaned. Every cleanup run
> selects it again, uses one slot of
> {{ozone.om.open.key.cleanup.limit.per.task}} for it and logs the rejected
> commit as an ERROR with a stack trace. No admin command clears it:
> {{recoverLease}} on the old name returns {{KEY_ALREADY_CLOSED}} or
> {{KEY_NOT_FOUND}}. Deleting it instead would send the renamed key's blocks
> for deletion, so cleaning it is left to a follow up, either a scan side skip
> or a removal that does not queue its blocks. Rejecting the rename of an
> hsync'ed key in OBJECT_STORE and LEGACY buckets removes this trigger for new
> renames and is proposed separately in HDDS-16849, which also covers a rename
> without a recreate (the scan then fails with a {{NullPointerException}}
> before this check is reached); this check also covers keys renamed before
> such a change. The scan hunk in the patch attached to HDDS-16768 would send
> such a leftover open key row (no key table entry) to {{DeleteOpenKeys}},
> which on OBJECT_STORE and LEGACY buckets queues the renamed key's blocks for
> deletion; limiting that hunk to FSO buckets, or skipping the row instead of
> deleting it, avoids that.
> Covered by {{testCommitWhenHsyncedKeyIsReplacedWithoutMarker}} in the
> existing {{TestOMKeyCommitRequest}}, inherited by
> {{TestOMKeyCommitRequestWithFSO}}. With the version finalized it checks that
> the close and a later hsync of the hsync'ed client are rejected and that a
> client whose open key carries a copied {{HSYNC_CLIENT_ID}} can still
> overwrite the name; before finalization it checks that the close is applied
> as before. Without the change the finalized case fails for both layouts with
> "expected: <KEY_NOT_FOUND> but was: <OK>". With it the commit request, commit
> response, recover lease, key rename, key delete, metadata manager, open key
> cleanup, OM storage, OM version and related request and response suites pass
> (248 tests), {{TestHSync}} passes on a mini cluster (42 tests, with both
> patches), and checkstyle is clean. With the patch the reproduction fails
> because {{k1}} keeps its block and nothing is queued for deletion.
> Found by TLA+ model checking and code review of the OM open key cleanup and
> hsync lease recovery paths under HDDS-15926, on commit
> a6b7bdb937109ba2893688b41a89470bc95c88cf. Checked against HDDS issues and
> apache/ozone pull requests for duplicates before filing; related but
> different are HDDS-10631 (block any open key rename) and HDDS-10770
> (overwrite of an hsync'ed key, which added {{OVERWRITTEN_HSYNC_KEY}};
> HDDS-10736 was closed as its duplicate). The attached patch is a proposal for
> review. Generated with Specula (Claude Opus 5.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]
