[
https://issues.apache.org/jira/browse/HDDS-16762?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Siyao Meng updated HDDS-16762:
------------------------------
Attachment: MC-6-check-destination-name-on-rename-to-bucket-root.patch
> FSO rename to the bucket root silently replaces or shadows an existing root
> entry and orphans its subtree
> ---------------------------------------------------------------------------------------------------------
>
> Key: HDDS-16762
> URL: https://issues.apache.org/jira/browse/HDDS-16762
> Project: Apache Ozone
> Issue Type: Bug
> Reporter: Siyao Meng
> Priority: Critical
> Attachments:
> MC-6-check-destination-name-on-rename-to-bucket-root.patch,
> TestBugMC6RenameToBucketRoot.java
>
>
> h3. Mechanism
> With an empty {{toKeyName}}, {{getOMKeyInfoIfExists}} returns null, so
> {{OMKeyRenameRequestWithFSO.validateAndUpdateCache}} takes the "destination
> absent" branch. {{renameKey}} then substitutes the source leaf name and puts
> {{/volId/bucketId/bucketId/<leaf>}} without looking that key up in
> {{directoryTable}} or {{fileTable}}. The existing destination checks (cases 3
> to 6) are skipped.
> When that key is also the key of the source's own parent (source {{p/p}}),
> the table cache applies parent then destination while the RocksDB batch in
> {{OMKeyRenameResponseWithFSO.addToDBBatch}} applies destination then parent,
> so cache and DB disagree.
> h3. Trigger
> One client renames an FSO entry to the bucket root while an entry with the
> same leaf name exists there: {{FileSystem.rename}} to {{/vol/bucket}} through
> {{ofs}} or to {{/}} through {{o3fs}} (both end up sending an empty
> destination key), or {{OzoneBucket.renameKey(from, "")}}.
> h3. Impact
> The rename returns OK and:
> * an existing root directory is replaced and its whole subtree becomes
> unreachable, with its rows left in RocksDB even after the bucket is deleted;
> * an existing root file is silently replaced;
> * a file can be stored under the name of a root directory and is unreachable;
> * renaming {{p/p}} to the root makes the renamed directory and its subtree
> disappear once the flush lands.
> The state survives an OM restart and nothing reclaims the orphan rows.
> h3. Reproduction
> PASS, deterministic, unmodified source. [^TestBugMC6RenameToBucketRoot.java]
> ({{hadoop-ozone/ozone-manager}}) drives the public bucket client against one
> real OM. Only the cache read before flush in the {{p/p}} variant uses
> {{OzoneManagerDoubleBuffer#pause}}. Files in the test are zero length, so
> block leakage is argued from the leftover {{fileTable}} rows, not measured.
> h3. Suggested fix
> Treat an empty {{toKeyName}} as the bucket root directory: look up the source
> leaf name directly under the bucket and fail with {{KEY_ALREADY_EXISTS}} when
> it is taken, the same rule case 5 applies to every other destination
> directory. Resolving the empty name to the leaf name before the existing
> destination lookup would be wrong: an existing root directory of that name
> would then be treated as "move the source under it".
> h3. Patch
> [^MC-6-check-destination-name-on-rename-to-bucket-root.patch], against
> 20e6a1c0f6039248373efd3eb1d7f2afcf7f1535. It also applies to master at
> c99d291e863.
> In the "destination absent" branch of
> {{OMKeyRenameRequestWithFSO.validateAndUpdateCache}}, an empty {{toKeyName}}
> now triggers a lookup of the source leaf name under the bucket (directory
> table, then file table, both cache aware) and fails with
> {{KEY_ALREADY_EXISTS}} if it is taken. A move into the bucket root with a
> free name is unchanged. A source already at the bucket root keeps its current
> behaviour, because existing tests rely on {{renameKey(rootKey, "")}}
> succeeding. With the check in place the cache and batch order difference in
> {{addToDBBatch}} is no longer reachable, so the patch does not reorder it.
> Covered by five cases in the existing {{TestOMKeyRenameRequestWithFSO}}.
> Without the change, {{testRenameToBucketRootWithExistingFile}},
> {{testRenameToBucketRootWithExistingDirectory}} and
> {{testRenameDirectoryToBucketRootOntoItsParent}} fail with "expected:
> <KEY_ALREADY_EXISTS> but was: <OK>". The rename and double buffer unit suites
> (48 tests) and the {{ofs}} / {{o3fs}} rename integration tests (23 tests,
> including {{testKeyRenameToBucketLevel}}) pass, checkstyle is clean. The
> patch prevents new damage only; rows already orphaned are not repaired.
> Found by TLA+ model checking and code review of the OM FSO namespace mutation
> paths under HDDS-15926, on commit 20e6a1c0f6039248373efd3eb1d7f2afcf7f1535.
> 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]
