[
https://issues.apache.org/jira/browse/HDDS-17467?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Siyao Meng updated HDDS-17467:
------------------------------
Attachment: TestBugCR11SnapshotMoveThenRenameOneBatch.java
> snapshotRenamedTable entry moved into the active DB from a deleted snapshot
> is overwritten by a rename response flushed in the same batch
> -----------------------------------------------------------------------------------------------------------------------------------------
>
> Key: HDDS-17467
> URL: https://issues.apache.org/jira/browse/HDDS-17467
> 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
> Priority: Major
> Attachments: CR-11-flush-snapshot-move-table-keys-in-own-batch.patch,
> TestBugCR11SnapshotMoveThenRenameOneBatch.java
>
>
> h3. Mechanism
> When the latest snapshot of a bucket is deleted, {{SnapshotDeletingService}}
> moves the entries of its {{snapshotRenamedTable}} out of the snapshot with
> SnapshotMoveTableKeys. Each entry maps {{/volume/bucket/objectID}} to the
> name the object has in the previous snapshot. With no next snapshot,
> {{OMSnapshotMoveTableKeysResponse.addToDBBatch}} puts these entries into the
> active DB {{BatchOperation}} on the double buffer flush thread. The request
> adds no cache entries for them. The legacy
> {{OMSnapshotMoveDeletedKeysResponse}} (SnapshotMoveDeletedKeys) puts moved
> entries the same way.
> {{OMKeyRenameResponse.addToDBBatch}},
> {{OMKeyRenameResponseWithFSO.addToDBBatch}} and
> {{OMKeysRenameResponse.addToDBBatch}} decide on the same thread whether to
> record a rename: they read the entry with {{snapshotRenamedTable.get}} and
> put their source name only if the read returns null. That read only returns
> data from batches that are already committed. When a rename response of an
> object comes after the move response in the same flush batch, the read misses
> the moved entry, returns null, and the rename puts its source name over it.
> The active DB entry then holds the object's name in the deleted snapshot
> instead of its name in the previous snapshot. Flushed one transaction at a
> time, the rename would read the moved entry and write nothing.
> 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 move response without a next snapshot carries an entry for an object, and
> the active DB has no entry of its own for that object when the move is
> flushed (the object was renamed between the previous snapshot and the deleted
> one, and not since).
> # A rename response for that object follows the move response in the same
> double buffer batch.
> A rename response before the move response in the same batch is harmless: the
> move's put comes later and overwrites it, as intended.
> h3. Impact
> * The deleted snapshot is purged once its tables are empty, and the previous
> snapshot is then the latest of the bucket.
> {{KeyManagerImpl.getPreviousSnapshotOzonePathInfo}} looks the object up in it
> under the stored name, for {{ReclaimableKeyFilter}} and
> {{ReclaimableDirFilter}}. The previous snapshot holds the object under its
> older name, so the lookup finds nothing. {{ReclaimableRenameEntryFilter}}
> also finds nothing under the stored name and may purge the entry. When the
> object is later deleted from the active bucket,
> {{ReclaimableKeyFilter.isReclaimable}} returns true and
> {{KeyDeletingService}} sends its blocks to SCM for deletion, although the
> previous snapshot still references them. The object ID check in
> {{ReclaimableKeyFilter.getPreviousSnapshotKeyInfo}} only runs when the lookup
> finds a key. Before the purge nothing is reclaimed, because the filter
> requires the previous snapshots in the chain to be active.
> * At this commit no service submits SnapshotMoveDeletedKeys, so it is reached
> only by replaying log entries written by a 1.4.x OM. In 1.4.0 and 1.4.1
> {{SnapshotDeletingService}} submitted it with the rename entries of the
> versions in the deleted snapshot's {{deletedTable}}. An overwrite keeps the
> object ID, so such a version can belong to an object that is still live and
> can be renamed, and the double buffer split only on CreateSnapshot. 1.4.0 and
> 1.4.1 are therefore affected in this narrower form (the object overwritten
> after its rename and before the deleted snapshot). For 1.4.x this is from
> reading the code, not run.
> h3. Reproduction
> [^TestBugCR11SnapshotMoveThenRenameOneBatch.java]
> ({{hadoop-ozone/ozone-manager}}) uses the snapshot response test base and a
> real {{OzoneManagerDoubleBuffer}}. The object is {{key}} in snapshot 1 and
> {{key_b}} in snapshot 2. It builds the real move response for snapshot 2 (no
> next snapshot) and a rename response {{key_b}} to {{key_c}}, and holds the
> flush with {{OzoneManagerDoubleBuffer.pause}} while both are added. On
> unmodified source the active DB entry is {{key_b}}, and
> {{KeyManagerImpl.getPreviousSnapshotOzoneKeyInfo}} for {{key_c}} finds
> nothing in snapshot 1, which holds the object as {{key}}. A control that
> flushes the two responses in separate batches gets {{key}}. A passing test
> means the defect is present.
> h3. Patch
> [^CR-11-flush-snapshot-move-table-keys-in-own-batch.patch], against
> a5814528cdc8d3c1b27f259e0db2256b585cb825. It also applies to master at
> 88119243f99.
> {{OzoneManagerDoubleBuffer.isStandaloneBatchCmdTypes}} now includes
> SnapshotMoveTableKeys and SnapshotMoveDeletedKeys, as it already does for
> CreateSnapshot and SnapshotPurge. The move is then committed in a batch of
> its own, so a rename response after it reads the moved entry from the
> committed DB and leaves it alone, and a rename response before it is still
> overwritten by the move. The stored value no longer depends on where the
> batch boundaries fall around a move. The split is per command type, so it
> covers the FSO and RenameKeys rename responses too. The cost is up to two
> extra RocksDB batch commits per move transaction. These transactions are
> submitted only while deleted snapshots are being cleaned up, and each one
> already commits and flushes the snapshot DBs it touches.
> The alternative of adding cache entries for the moved entries at apply time
> was not taken. It would make the entries visible to readers before they are
> committed and adds cache handling to the move request, for the same result.
> {{TestOMSnapshotMoveTableKeysResponse.testRenameAfterMoveToActiveDbKeepsMovedRenameEntry}}
> flushes a real move response (either type) and a rename response of the same
> object through one {{OzoneManagerDoubleBuffer}} flush, in both orders, and
> asserts that the active DB entry names the object in the remaining snapshot.
> Without the change both move first cases fail (the entry holds the name from
> the deleted snapshot). With it, the snapshot move, create, delete and purge
> suites, the rename response suites, the double buffer suites and
> {{TestOzoneManagerStateMachine}} and {{TestSnapshotDeletingService}} pass,
> and checkstyle is clean. Integration tests were not run.
> Found by code review of the OM table cache, double buffer flush and startup
> paths, as part of the TLA+ verification effort under HDDS-15926, on commit
> a5814528cdc8d3c1b27f259e0db2256b585cb825. 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
(v10.3.26#10030026)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
