[
https://issues.apache.org/jira/browse/HDDS-17468?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Siyao Meng reassigned HDDS-17468:
---------------------------------
Assignee: Siyao Meng
> Rename responses miss an earlier snapshotRenamedTable put in the same flush
> batch, so blocks a snapshot still holds can be reclaimed
> ------------------------------------------------------------------------------------------------------------------------------------
>
> Key: HDDS-17468
> URL: https://issues.apache.org/jira/browse/HDDS-17468
> Project: Apache Ozone
> Issue Type: Bug
> Affects Versions: 1.4.0, 2.0.0, 1.4.1, 2.1.0, 2.2.0, 2.1.1, 2.1.2, 2.2.1
> Reporter: Siyao Meng
> Assignee: Siyao Meng
> Priority: Critical
> Attachments: MC-2-record-first-rename-once-per-flush-batch.patch
>
>
> h3. Mechanism
> In a bucket that has a snapshot, {{snapshotRenamedTable}} maps
> {{/volume/bucket/objectID}} to the name the object has in the latest
> snapshot, and only the first rename after the snapshot may write it.
> {{OMKeyRenameResponse.addToDBBatch}}, {{OMKeysRenameResponse.addToDBBatch}}
> and {{OMKeyRenameResponseWithFSO.addToDBBatch}} enforce that on the double
> buffer flush thread: each reads the entry with {{snapshotRenamedTable.get}}
> and puts its source name only if the read returns null.
> Nothing adds cache entries to {{snapshotRenamedTable}}, so that read only
> returns data from batches that are already committed. A put that an earlier
> response added to the current {{BatchOperation}} is not visible to it. When
> two rename responses for the same object are in one flush batch, the second
> one also reads null and its put replaces the first one's. The entry then
> holds the intermediate name, which the object never had in the snapshot,
> instead of its name in the snapshot.
> Which value is stored depends on where the batch boundaries fall, so two OMs
> that apply the same log can store different values for the same entry.
> h3. Trigger
> # A bucket has a snapshot, and an object in it has no
> {{snapshotRenamedTable}} entry yet.
> # Two rename responses for that object are flushed in the same double buffer
> batch, the second one renaming from the name the first one produced.
> # The object's key is deleted and {{KeyDeletingService}} processes its
> {{deletedTable}} entry.
> h3. Impact
> * {{KeyManagerImpl.getPreviousSnapshotOzonePathInfo}} looks the deleted key
> up in the snapshot under the stored intermediate name and finds nothing.
> {{ReclaimableKeyFilter.isReclaimable}} returns true, and
> {{KeyDeletingService}} purges the entry and sends its blocks to SCM for
> deletion, although the snapshot still holds the key under its original name
> with the same object ID and blocks. The object ID check in
> {{ReclaimableKeyFilter.getPreviousSnapshotKeyInfo}} runs only when the lookup
> finds a key, so it does not catch this. Once the datanodes delete those
> blocks, the key can no longer be read from the snapshot. That last step was
> not run: the test setup has no datanodes.
> * Nothing corrects the wrong entry later. The next snapshot of the bucket
> copies it through its checkpoint, so the snapshot deep clean path reads it as
> well. This was found by reading the code, not run.
> h3. Reproduction
> The regression tests in the attached patch cover it, and the ones marked
> below fail on unmodified source. All are in {{hadoop-ozone/ozone-manager}}:
> * {{TestOMKeyRenameResponse.testSecondRenameKeepsSnapshotName}}, also run
> through {{TestOMKeyRenameResponseWithFSO}}: two rename responses for one key
> in a snapshotted bucket. With both in one batch it fails without the change
> (the entry holds the intermediate name). With a commit between them it
> passes, as a control.
> * {{TestOMKeysRenameResponse.testSecondRenameInSameBatchKeepsSnapshotName}}:
> two {{OMKeysRenameResponse}} instances for one key in one batch. Fails
> without the change.
> *
> {{TestOMKeyRenameResponseWithFSO.testSecondDirectoryRenameInSameBatchKeepsSnapshotName}}:
> two directory rename responses in one batch. Fails without the change.
> *
> {{TestOMKeyRenameResponse.testRenameOfObjectNotCommittedAtSourceRecordsNothing}},
> also run through {{TestOMKeyRenameResponseWithFSO}}: the committed source
> name holds a different object, so no entry may be written. Without the change
> an entry is written.
> There is no standalone reproduction.
> h3. Patch
> [^MC-2-record-first-rename-once-per-flush-batch.patch], against
> a5814528cdc8d3c1b27f259e0db2256b585cb825. It also applies to master at
> 88119243f99.
> The decision stays on the flush thread, and all three responses get one more
> condition: the committed DB must still hold the same object (same object ID)
> at the rename's source key. The check reads with {{getSkipCache}} from the
> key table, the file table, or for an FSO directory rename the directory table
> (helper {{OMKeyRenameResponse.isCommittedAt}}). If an earlier rename of the
> object is in the same batch, the object got its current name inside that
> batch. The committed table then does not hold it at the source, and the later
> rename leaves the entry that the first rename wrote. So for renames of the
> same object, the stored value no longer depends on where the batch boundaries
> fall.
> The decision is deliberately not moved to apply time. Snapshot create clears
> the bucket's {{snapshotRenamedTable}} range only when its own batch is
> flushed, and it adds no cache entries. A rename applied after a snapshot
> create but before that flush would read the stale entry, write nothing, and
> leave the object without an entry once the range is cleared. That loses
> blocks the same way. On the flush thread, snapshot create is already a batch
> of its own, so a rename after it always reads the cleared range.
> A separate, older defect is not addressed here: an
> {{OMSnapshotMoveTableKeysResponse}} that moves entries into the active DB,
> followed in the same batch by a rename of one of those objects, still
> overwrites the moved entry. That is HDDS-17467, whose patch is independent of
> this one.
> The patch adds no request or response field and no proto change. Its cost is
> one extra point read per rename, only in a bucket that has a snapshot and
> only while the object has no entry. An object that is created and renamed
> within one batch now gets no entry. It is in no snapshot, so that entry was
> never used. Entries already stored with a wrong name are not repaired.
> With the patch, these pass: the rename, key purge, directory purge, snapshot
> create and snapshot move table keys response and request suites,
> {{TestKeyManagerImpl}}, {{TestKeyDeletingService}},
> {{TestSnapshotDeletingService}}, {{TestReclaimableKeyFilter}} and
> {{TestReclaimableRenameEntryFilter}}. Checkstyle is clean. Integration tests
> were not run.
> Found by TLA+ model checking and code review of the OM table cache, double
> buffer flush and startup paths under HDDS-15926, on commit
> a5814528cdc8d3c1b27f259e0db2256b585cb825. Checked against HDDS issues and
> apache/ozone pull requests for duplicates before filing. Related but not
> duplicates: HDDS-8189 and HDDS-8435 (introduced the check), HDDS-17173 (table
> iterators do not return unflushed entries), HDDS-15125 (proposed redesign of
> snapshot reclaimability). The attached patch is a proposal for review.
> Generated with Specula (Claude Opus 5.5).
--
This message was sent by Atlassian Jira
(v10.3.26#10030026)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
