Skip to content

HDDS-16951. Remove the per-call InfoBucket from rooted-ofs rename on FSO buckets - #11674

Open
yandrey321 wants to merge 5 commits into
apache:masterfrom
yandrey321:HDDS-16951
Open

yandrey321 wants to merge 5 commits into
apache:masterfrom
yandrey321:HDDS-16951

Conversation

@yandrey321

Copy link
Copy Markdown
Contributor

What changes were proposed in this pull request?

Problem. Rooted-ofs rename fetches the bucket (InfoBucket, plus InfoVolume + InfoBucket per link hop)
only to choose the algorithm: FSO sends one RenameKey, LEGACY iterates over the keys, OBJECT_STORE is rejected on
the client. So every FSO rename costs 2 OM requests. OM cannot simply reject non-FSO buckets in RenameKey: the
shell, trash, key lifecycle and OzoneBucket.renameKey send it on any layout, and on LEGACY it renames one key.

Implementation (same shape as HDDS-16760 / HDDS-16948):

  • RenameKeyRequest gets optional bool requireFileSystemOptimized = 4. When it is set,
    OMKeyRenameRequest.preExecute (inherited by the FSO request) rejects any bucket whose link-resolved layout is
    not FILE_SYSTEM_OPTIMIZED with NOT_SUPPORTED_OPERATION, before Ratis. It tests the layout, not the request class,
    because the OBJECT_STORE request class also serves LEGACY. Unlike requireFileSystemLayout, it rejects LEGACY too.
  • New ClientProtocol.renameKey(..., requireFileSystemOptimized); the existing overload is unchanged.
  • New OzoneManagerVersion.RENAME_KEY_REJECTS_NON_FSO(104). From that version on, BasicRootedOzoneFileSystem.rename
    first sends the flagged RenameKey with no getBucket(). Success and KEY_ALREADY_EXISTS / KEY_RENAME_ERROR /
    KEY_NOT_FOUND are handled exactly as in today's FSO rename. NOT_SUPPORTED_OPERATION, BUCKET_NOT_FOUND and
    VOLUME_NOT_FOUND fall back to today's path from getBucket() on, so LEGACY renames, OBJECT_STORE rejections and
    missing-bucket errors are unchanged. Older OMs, including ZDU, keep today's path.

Side effects.

  • FSO: 2 -> 1 OM request per rename. LEGACY and OBJECT_STORE: one extra rejected RenameKey before today's path
    (the audit log shows a failed RENAME_KEY before INFO_BUCKET).
  • Callers that do not set the field (old clients, shell, trash, lifecycle, OzoneBucket.renameKey) are unaffected.

Generated-by: Claude Code (Claude Opus 5.5)

What is the link to the Apache JIRA

https://issues.apache.org/jira/browse/HDDS-16951

How was this patch tested?

Unit and integration tests

Benchmark
CPU savings. CPU profiles of the 10-thread run show the patch saves about 110 us of CPU per FSO rename: about
67 us on the client (46% of its CPU per rename) and about 44 us on the OM (about 20% of its request-handling CPU; the
write pipeline is unchanged). Almost all of it is per-RPC overhead, not OM work: the InfoBucket request itself costs
about 2 us. It does not show up as throughput because the benchmark is bound by the OM's Ratis log fsync, not CPU.

@smengcl smengcl left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1

@chungen0126 chungen0126 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @yandrey321 for working on this. Overall LGTM, just left two minor comments.

* file system rename can be sent without fetching the bucket to learn its layout.
*/
boolean isRenameRequiringFileSystemOptimizedSupported() {
return proxy.getOmVersion().compareTo(OzoneManagerVersion.RENAME_KEY_REJECTS_NON_FSO) >= 0;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please use ComponentVersion#isSupportedBy.

Comment on lines +79 to +86
// Checked before Ratis only: preExecute does not run when the request is applied. The request was created for the
// link-resolved layout, and the OBJECT_STORE request class also serves LEGACY, so test the layout itself.
if (renameKeyRequest.getRequireFileSystemOptimized() && !getBucketLayout().isFileSystemOptimized()) {
throw new OMException("Bucket: " + renameKeyRequest.getKeyArgs().getBucketName()
+ " has layout: " + getBucketLayout() + ", but the rename requires "
+ BucketLayout.FILE_SYSTEM_OPTIMIZED + ".", OMException.ResultCodes.NOT_SUPPORTED_OPERATION);
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not directly related to this PR, just noting down for the record: if the operation fails in preExecute, it won't be recorded in the audit log.

@chungen0126 chungen0126 Oct 24, 2026 •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Created HDDS-17382 for follow-up.

@chungen0126

Copy link
Copy Markdown
Contributor

@yandrey321 Please rebase to fix the failing CI checks.

@yandrey321

Copy link
Copy Markdown
Contributor Author

Rebased and updated code to adjust for changes in the master

@chungen0126 chungen0126 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1 LGTM

Change-Id: Ib34f5c3cbd765bfa86b39de5ce478f9ab0b5e8cb
@jojochuang

Copy link
Copy Markdown
Contributor

The integration (client) failure is caused by the newly added OM version exposing an equality check in TestRpcClient.testIfNoneMatchETagReachesOMOrRejectsOldVersion.

The test currently expects If-None-Match support only when version == S3_IF_NONE_MATCH_ETAG (104). Production RpcClient.checkIfNoneMatchETagSupport() correctly accepts 104 and later. With this PR's RENAME_KEY_REJECTS_NON_FSO (105), the three requests therefore reach the mock OM, but the test incorrectly expects an empty request list at line 695.

The fix is to use version.compareTo(OzoneManagerVersion.S3_IF_NONE_MATCH_ETAG) >= 0 in the test, matching the production capability check. This also handles future OM versions.

I pushed the one-line fix on top of this PR's head to my fork: jojochuang@f6cfeb8 (branch codex/hdds-16951-ci-fix). Feel free to cherry-pick it.

Validation: git diff --check passes. I attempted the targeted parameterized test on JDK 21, but compilation stopped before tests because local Ozone snapshot dependencies are stale (missing setExcludedETag and setRequireFileSystemOptimized); I have not verified a passing local test run.

The integration (ozone) failure is different: TestBackgroundContainerDataScannerIntegration reports an RDBMetrics source still registered after shutdown. I found no evidence linking that failure to the rename change.

@adoroszlai

Copy link
Copy Markdown
Contributor

The integration (client) failure is caused by the newly added OM version exposing an equality check in TestRpcClient.testIfNoneMatchETagReachesOMOrRejectsOldVersion.

@chungen0126 Please check CI run in fork before triggering CI in Apache. The same test failed in fork.

I pushed the one-line fix on top of this PR's head to my fork: jojochuang@f6cfeb8 (branch codex/hdds-16951-ci-fix). Feel free to cherry-pick it.

There is no need to cherry-pick it, because it was pushed 2 minutes later to the PR branch.

The integration (ozone) failure is different: TestBackgroundContainerDataScannerIntegration reports an RDBMetrics source still registered after shutdown. I found no evidence linking that failure to the rename change.

This one was fixed on master (2b26179), can be ignored for this PR.

…icts

Preserve MULTIPART_PART_IV at version 105 and assign RENAME_KEY_REJECTS_NON_FSO version 106.

Co-authored-by: Codex (GPT-6)
Change-Id: I9dd88ffbb1ab4523dc0b85f0feaf0114afe694d2
@jojochuang

Copy link
Copy Markdown
Contributor

CI is green in the fork: https://github.com/yandrey321/ozone/actions/runs/40967890669

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants