Skip to content

HDDS-15925. Remove redundant InfoBucket RPC from OFS getFileStatus by validating bucket layout server-side - #11226

Open
yandrey321 wants to merge 4 commits into
apache:masterfrom
yandrey321:HDDS-15925-alt
Open

HDDS-15925. Remove redundant InfoBucket RPC from OFS getFileStatus by validating bucket layout server-side#11226
yandrey321 wants to merge 4 commits into
apache:masterfrom
yandrey321:HDDS-15925-alt

Conversation

@yandrey321

@yandrey321 yandrey321 commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

What changes were proposed in this pull request?

OFS getFileStatus on a non-snapshot path used to issue two OM RPCs per call:
an InfoBucket RPC (only so the client could read the bucket layout and reject
OBJECT_STORE buckets, which have no file system semantics) followed by the actual
getFileStatus RPC. On a namespace-walk / listing-heavy workload this doubles the
OM read-RPC volume for no functional gain.

This PR removes the redundant InfoBucket RPC by moving the layout check to where
the authoritative bucket metadata already lives — the OM:

  • Client (BasicRootedOzoneClientAdapterImpl) — the non-snapshot getFileStatus
    path now calls proxy.getOzoneFileStatus(...) directly instead of first fetching
    the bucket. Mutating OFS operations still resolve and validate layout through
    getBucket(...), and snapshot paths are unchanged.
  • Server (OmMetadataReader.getFileStatus) — validates the bucket layout and
    rejects OBJECT_STORE buckets. The IllegalArgumentException from
    OzoneFSUtils.validateBucketLayout is wrapped as
    OMException(NOT_SUPPORTED_OPERATION) so it is returned as a normal
    (non-retryable) RPC response instead of escaping the read handler's IOException
    catch and triggering a client-side retry storm.
  • Client error mapping — on NOT_SUPPORTED_OPERATION the adapter re-throws
    IllegalArgumentException (preserving the pre-existing OFS behavior and message),
    keyed on the ResultCode rather than on the message text so the two sides are not
    coupled through a string.
  • Rolling-upgrade version gate — the direct call is only taken when the
    negotiated OM version is new enough to perform the server-side check; against an
    older OM the client falls back to the pre-HDDS-15925 path (see the compatibility
    note below).
  • getTrashRoots skips OBJECT_STORE bucketsgetTrashRoots(allUsers=true)
    iterates every volume/bucket and probes each bucket's trash path with exists().
    OBJECT_STORE buckets have no file system semantics (hence no trash root), and
    getFileStatus on them now surfaces IllegalArgumentException, which exists()
    does not swallow and which would abort the whole scan. The scan now skips
    OBJECT_STORE buckets up front (reusing the OzoneBucket already listed, no extra
    RPC). This also hardens the trash emptier against clusters that contain OBS
    buckets, which previously could throw out of getTrashRoots.

No protobuf/wire change: RPC signatures and the GetFileStatus messages are
unchanged, and NOT_SUPPORTED_OPERATION is a pre-existing result code.

Compatibility note

The OBS-rejection check moves from the client to the OM. In a rolling upgrade, a
new OFS client talking to an old OM would otherwise stop rejecting
getFileStatus on an OBJECT_STORE bucket, because the old OM has no server-side
check and the new client no longer performs the InfoBucket-based one.

This is now handled with a version gate on the already-negotiated OM version:

  • A new OzoneManagerVersion.GET_FILE_STATUS_REJECTS_OBS marks the server version
    that performs the OBS rejection. This is an additive Java enum constant carried
    over the existing OMVersion int in ServiceInfono protobuf/wire change;
    unknown values still map to FUTURE_VERSION.
  • ClientProtocol.getOmVersion() exposes the negotiated version (in HA this is the
    minimum across all OMs, so a single old OM forces the safe path), implemented
    by RpcClient from the version it already captures during handshake.
  • BasicRootedOzoneClientAdapterImpl.getFileStatusForKeyOrSnapshot gates on it:
    when the OM is >= GET_FILE_STATUS_REJECTS_OBS it issues the direct
    getOzoneFileStatus call and relies on the server-side rejection; otherwise it
    falls back to fetching the bucket and validating its layout client-side — the
    exact pre-HDDS-15925 behavior, including rejecting OBS with
    IllegalArgumentException.

New-client/new-OM takes the optimized single-RPC path; new-client/old-OM keeps the
old two-RPC behavior with the client-side check; old-client/* is unaffected.

Generated-by: Claude Code (Claude Opus 4.8)

What is the link to the Apache JIRA

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

How was this patch tested?

clean CI run: https://github.com/yandrey321/ozone/actions/runs/34513864283

  • Unit: TestOMMetadataReader (server-side rejection returns
    OMException(NOT_SUPPORTED_OPERATION), keyManager.getFileStatus not called for
    OBS), TestBasicRootedOzoneClientAdapterHeadOp (8/8).
  • Integration: TestOFS#testGetFileStatusRejectsObsBucket — verifies OBS
    getFileStatus is rejected over RPC as IllegalArgumentException with 0
    InfoBucket RPCs; the OFS getFileStatus suite in AbstractRootedOzoneFileSystemTest.
  • Benchmark: TestOfsGetFileStatusCacheBenchmark (tagged benchmark), numbers above.
  • checkstyle.sh clean; affected modules build.

Benchmark

Measured with TestOfsGetFileStatusCacheBenchmark (200 buckets × 10 accesses =
2000 getFileStatus calls), same base and machine, against baseline and the
client-side cache alternative (PR #11176):

Metric Baseline #11176 (client cache) This PR (server-side)
InfoBucket RPCs 2000 200 0
getFileStatus RPCs 2000 2000 2000
Single-thread p99 0.32–0.48 ms 0.29–0.37 ms 0.22–0.28 ms
Single-thread throughput ~3.7–4.5k ops/s ~5.4–6.5k ops/s ~6.1–7.2k ops/s
10-thread p99 1.033 ms 0.818 ms 0.719 ms
10-thread throughput 16.2k ops/s 26.9k ops/s 26.1k ops/s

The getFileStatus RPC count is identical everywhere — the change removes only the
redundant InfoBucket RPC. This PR removes it entirely (0), giving the lowest RPC
count and the best latency tail with no client-side cache state or new config.

@yandrey321

Copy link
Copy Markdown
Contributor Author

@jojochuang please check this PR based on #10792, we need to decide if we want to go with server side approach vs client side cache: #11176

@sodonnel

Copy link
Copy Markdown
Contributor

The OBS-rejection check moves from the client to the OM. In a rolling upgrade, a
new OFS client talking to an old OM will no longer reject getFileStatus on an
OBJECT_STORE bucket, because the old OM has no server-side check and the new client
no longer performs the InfoBucket-based one. New-client/new-OM and old-client/*
are unaffected. Flagging for reviewer sign-off on whether that upgrade window needs
a version gate or a retained client-side fallback.

Thanks for including that detail here. I think this would need a version gate, and it should be fairly simple. The client is already aware of the server version as it gets it on the first RPC / handshake so we can just do:

if (server.version < NEW_FEATURE) {
  old behavior
} else {
  new behavior
}

I haven't been involved in this and I haven't looked at the code here beyond the description, but I would tend to prefer a solution that doesn't have a client side cache, as caching always proves more tricky than it first seems!

@yandrey321

Copy link
Copy Markdown
Contributor Author

The OBS-rejection check moves from the client to the OM. In a rolling upgrade, a
new OFS client talking to an old OM will no longer reject getFileStatus on an
OBJECT_STORE bucket, because the old OM has no server-side check and the new client
no longer performs the InfoBucket-based one. New-client/new-OM and old-client/*
are unaffected. Flagging for reviewer sign-off on whether that upgrade window needs
a version gate or a retained client-side fallback.

Thanks for including that detail here. I think this would need a version gate, and it should be fairly simple. The client is already aware of the server version as it gets it on the first RPC / handshake so we can just do:

if (server.version < NEW_FEATURE) {
  old behavior
} else {
  new behavior
}

I haven't been involved in this and I haven't looked at the code here beyond the description, but I would tend to prefer a solution that doesn't have a client side cache, as caching always proves more tricky than it first seems!

implemented version check on the client side.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants