Siyao Meng created HDDS-17468:
---------------------------------
Summary: 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
Reporter: Siyao Meng
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]
