[
https://issues.apache.org/jira/browse/HDDS-16851?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Siyao Meng updated HDDS-16851:
------------------------------
Attachment: MC-2-overwrite-open-key-records-open-time.patch
> Open key cleanup deletes an overwrite in progress when the overwritten key is
> older than the expire threshold, so its commit fails
> ----------------------------------------------------------------------------------------------------------------------------------
>
> Key: HDDS-16851
> URL: https://issues.apache.org/jira/browse/HDDS-16851
> Project: Apache Ozone
> Issue Type: Bug
> Reporter: Siyao Meng
> Priority: Critical
> Attachments: MC-2-overwrite-open-key-records-open-time.patch
>
>
> h3. Mechanism
> When a key is overwritten, {{OMKeyRequest.prepareFileInfo}} builds the open
> key from the existing key with {{dbKeyInfo.toBuilder()}}. It replaces the
> modification time, size, metadata, tags and update ID, but keeps the creation
> time of the existing key. {{OmMetadataManagerImpl.getExpiredOpenKeys}}
> selects an open key that is not hsync'ed when its creation time is older than
> {{ozone.om.open.key.expire.threshold}} (default 7d), and
> {{OMOpenKeysDeleteRequest}} only checks that the open key still exists. So
> when the existing key is older than the threshold, the next
> {{OpenKeyCleanupService}} run deletes the open key of the overwrite, however
> recently it was opened. This applies to createKey and createFile in OBS,
> LEGACY and FSO buckets, which all go through {{prepareFileInfo}}.
> Keeping the creation time is intended for the committed key. An overwrite or
> rewrite keeps the original creation time of the committed key (asserted since
> HDDS-10527), and the committed key takes it from the open key. Only its use
> as the age of the open key is wrong. The description of
> {{ozone.om.open.key.expire.threshold}} also says "if a key has been open
> longer than" the threshold.
> Present since 1.3.0, where {{OpenKeyCleanupService}} (HDDS-4123) started to
> age open keys by creation time.
> h3. Trigger
> # Key k is committed.
> # More than {{ozone.om.open.key.expire.threshold}} later, a client opens k
> again (createKey or createFile) and allocates a block, without hsync.
> # A cleanup run happens before the client closes the key.
> No fault is needed. With the defaults (cleanup every 24h, threshold 7d), any
> overwrite of a key older than 7 days that is still open at a cleanup run is
> affected, so long writes are the most exposed. A writer that uses hsync is
> exposed until its first hsync.
> h3. Impact
> * The open key is deleted and the blocks of the new write are put in
> {{deletedTable}}.
> * The next allocateBlock, hsync or commit of the writer fails with
> {{KEY_NOT_FOUND}} (for the commit: "entry is not found in the OpenKey
> table"). The data of that write is lost and has to be written again. On a
> bucket without versioning the previous version of the key is not changed.
> * On a bucket with versioning enabled, the open key of the overwrite also
> carries the block versions of the live key (the {{prepareFileInfo}} branch
> described in HDDS-16732), so the cleanup queues the committed key's blocks
> for deletion while the key table still references them: committed data of
> that key is lost. Reproduced on the same commit at the OM level. Versioning
> can only be enabled through the Java client API.
> h3. Reproduction
> PASS on unmodified source. [^TestBugMC2OverwriteOpenKeyExpiredEarly.java]
> ({{hadoop-ozone/ozone-manager}}) drives one real OM through
> {{OzoneManagerProtocol}}, with the testing SCM block client and no datanodes.
> The open key expiry is shortened to 2 s and the service interval is set to
> one hour, and the test runs one cleanup pass with {{runPeriodicalTaskNow}}.
> It commits key1, waits until key1 is older than the threshold, then opens an
> overwrite of key1 and a new key2 at the same moment. The cleanup pass, a few
> milliseconds later, deletes only the overwrite and queues its block for
> deletion. The commit of the overwrite then fails with {{KEY_NOT_FOUND}}, key1
> keeps its old size, and key2 commits normally. The only timing dependency is
> the 2 s wait. A passing test means the defect is present.
> h3. Patch
> [^MC-2-overwrite-open-key-records-open-time.patch], against
> a6b7bdb937109ba2893688b41a89470bc95c88cf. It also applies to master at
> b0aa6475b78.
> The open key of an overwrite now records its open time as creation time, the
> same value a new key gets. {{OMKeyCommitRequest}} and
> {{OMKeyCommitRequestWithFSO}} set the creation time of the committed key from
> the key it replaces, so an overwrite or rewrite still keeps the original
> creation time. {{ozone admin om list-open-files}} now shows the open time for
> an overwrite, not the creation time of the overwritten key. Changing the
> expiry check to use the modification time was not chosen, because
> {{OMAllocateBlockRequest}} updates it, which would change expiry for every
> open key.
> Covered by {{testOverwriteOfOldKeyIsNotExpiredOpenKey}} in
> {{TestOMKeyCreateRequest}} and an extra assertion in
> {{TestOMKeyCommitRequest.testAtomicRewrite}}, both inherited by the FSO
> subclasses. Without the change both fail for both layouts.
> {{TestOMKeyCommitRequest.testValidateAndUpdateCache}} now also asserts that a
> new key keeps the creation time of its open key. Three existing assertions in
> {{TestOMKeyCreateRequest}} that expected the open key to carry the old
> creation time are updated. With the change, these all pass and checkstyle is
> clean:
> * the key create, commit, file create, allocate block, delete, rename, ACL,
> recover lease, purge and open key delete request suites,
> {{TestOmMetadataManager}} and {{TestOpenKeyCleanupService}} (441 tests)
> * the rewrite tests of {{TestOzoneRpcClient}} on a mini cluster (15 tests)
> With the patch the reproduction fails, because the open key of the overwrite
> now carries its own open time.
> The committed creation time now comes from the key present at commit time,
> not the key present when the write was opened, as
> {{S3MultipartUploadCompleteRequest}} already does. The two differ only if the
> key is deleted, or created by another writer, between open and commit. Open
> keys written before the upgrade keep the inherited creation time, so the
> first cleanup run after the upgrade can still delete such an overwrite in
> progress. No version gate is added: during a rolling upgrade, OMs on the old
> and new version store different creation times in the open key row of an
> overwrite (the committed key gets the same value on both except in the corner
> case above). Reviewers may prefer to gate it.
> 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. 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]
