yandrey321 opened a new pull request, #11380:
URL: https://github.com/apache/ozone/pull/11380
## What changes were proposed in this pull request?
`BasicRootedOzoneClientAdapterImpl.getFileChecksum` issues three OM RPCs per
call:
`InfoVolume`, `InfoBucket`, and `LookupKey`. Through a link bucket it is
five, because the link
resolution repeats the volume and bucket lookups. Only the key lookup
carries information the
checksum needs — the volume and bucket objects were used solely for
`getName()`.
The `InfoBucket` call existed only to reject OBJECT_STORE buckets, which
have no file system
semantics. This patch moves that check to the server, where the bucket is
already resolved, and the
other two RPCs then have no reason to exist.
### Server side
`OmMetadataReader.lookupFile` now validates the resolved bucket layout and
rejects OBJECT_STORE,
mirroring what HDDS-15925 (#11226) did for `getFileStatus`.
`OzoneFSUtils.validateBucketLayout`'s
`IllegalArgumentException` is wrapped as
`OMException(NOT_SUPPORTED_OPERATION)` so it reaches the
client as a normal, non-retryable RPC response instead of escaping the read
handler's `IOException`
catch and triggering a client retry storm.
`lookupFile` is the right home for the check: it is the file system twin of
`lookupKey`, it already
resolves the bucket, and it previously performed no layout validation at
all. `lookupKey` cannot take
the check, because it is shared with OBS and S3 reads.
`lookupFile` is otherwise equivalent to `lookupKey` for these arguments —
both honor
`getLatestVersionLocation()`, call `refresh()` and `sortDatanodes()`, check
`ResourceType.KEY`/`READ`,
and apply `normalizeKeyArgs(bucket.update(args), bucket)`. Both also call
`addBlockToken4Read`, which
is why the checksum cannot instead be routed through `getFileStatus`:
`KeyManagerImpl.getOzoneFileStatus{,FSO}` never adds block tokens, so the
returned locations would be
unusable in secure mode.
### Client side
`OzoneClientUtils.getFileChecksumWithCombineMode` now takes `(volumeName,
bucketName, keyName, …)`
plus a `useLookupFile` flag instead of `OzoneVolume`/`OzoneBucket`. The
name-based signature is pushed
down through `ChecksumHelperFactory`, `BaseFileChecksumHelper` and both
subclasses, which removes the
implicit coupling that the helpers may only ever call `getName()` on those
objects.
The ofs adapter gates on the negotiated OM version:
```java
boolean omRejectsObs = proxy.getOmVersion()
.compareTo(OzoneManagerVersion.LOOKUP_FILE_REJECTS_OBS) >= 0;
```
New OM: one `LookupFile`. Pre-upgrade OM (a new client against an older
server during a rolling
upgrade): keep the client-side layout check and use `LookupKey` — still two
RPCs rather than three,
because the `InfoVolume` call is dropped unconditionally.
`OzoneManagerUtils.reportNotFound` probes
the volume table and raises `VOLUME_NOT_FOUND` before falling back to
`BUCKET_NOT_FOUND`, so dropping
it loses no error fidelity. The fallback retires itself as clusters upgrade.
`NOT_SUPPORTED_OPERATION` is mapped back to `IllegalArgumentException`, so
the user-visible behavior
for an OBS bucket is unchanged — the same mapping #11226 uses for
`getFileStatus`. `NOT_A_FILE`,
which is how `lookupFile` reports a directory where `lookupKey` reported
`KEY_NOT_FOUND`, becomes
`FileNotFoundException`, matching HDFS.
`OzoneManagerVersion.LOOKUP_FILE_REJECTS_OBS(15, …)` is additive. There is
no protobuf change.
o3fs (`BasicOzoneClientAdapterImpl`) and `ozone sh key checksum`
deliberately stay on `LookupKey`:
o3fs validates the layout once in its constructor and has no `InfoBucket` to
save on this path, and
the CLI must keep working against OBS buckets.
### Relation to [PR 11369](https://github.com/apache/ozone/pull/11369)
[PR 11369](https://github.com/apache/ozone/pull/11369) addresses the same
three RPCs from the client side: it drops the `InfoVolume` call and adds a
per-bucket layout cache (`ozone.client.fs.bucket.layout.cache.expiry`,
default 2m;
`ozone.client.fs.bucket.layout.cache.size`, default 1000) so the
`InfoBucket` call is paid once per
bucket instead of once per call.
The two are not in conflict, and #11369's first half — dropping
`InfoVolume`, which is pure
redundancy — is carried here. They differ in what the remaining `InfoBucket`
cost becomes:
| | master | #11369 (client cache) | this PR (server side) |
|---|---|---|---|
| OM RPCs per call | 3.00 | 2 − hitRatio | **1.00** |
| …at a 90% hit ratio | 3.00 | 1.10 | **1.00** |
| …cold client, or more buckets than the cache holds | 3.00 | 2.00 |
**1.00** |
| Staleness window | none | 2 min (default) | none |
| New configuration | none | 2 keys | none |
| Client memory | none | up to 1000 layouts | none |
| Works against an un-upgraded OM | — | yes | falls back to 2.00 |
At the hit ratio #11369 is designed for, the RPC counts are close (1.10 vs
1.00) and so is the
measured latency. The case for doing this server-side is that 1.00 is
unconditional: there is no
hit-ratio cliff for a cold client or a bucket count above the cache size, no
window in which a
deleted-and-recreated bucket resolves against a stale layout, and nothing to
configure or invalidate.
The cost is that the single-RPC path needs the OM upgraded; until then the
client falls back to two
RPCs, which is still better than master.
A secondary benefit of putting the check in `lookupFile` is that it is not
client-specific: any
caller that reaches `lookupFile` on an OBS bucket now gets a clear
`NOT_SUPPORTED_OPERATION` instead
of file system semantics silently applied to a bucket that has none.
Workload adapted from #11369's own benchmark so all three arms run identical
test code:
2000 `getFileChecksum` calls over 200 FSO buckets, 10 accesses per bucket
from a seed-shuffled
sequence (a 90% cache-hit workload — #11369's best case), 4 KiB files, 3
datanodes,
`MiniOzoneCluster` on loopback. Each arm was built and installed separately
and run 3 times;
figures are medians of 3.
### OM RPCs per call — exact, identical in all 9 runs
| | master | #11369 | this PR |
|---|---|---|---|
| InfoVolume | 2000 | 0 | **0** |
| InfoBucket | 2000 | 200 | **0** |
| LookupKey | 2000 | 2000 | 0 |
| LookupFile | 0 | 0 | 2000 |
| **per call** | **3.00** | **1.10** | **1.00** |
This is deterministic rather than a timing measurement. It also confirms the
version gate negotiates
correctly, since the single-RPC path is taken only when the OM advertises
`LOOKUP_FILE_REJECTS_OBS`.
### Latency and throughput (medians of 3)
Single-threaded:
| | master | #11369 | this PR | vs master | vs #11369 |
|---|---|---|---|---|---|
| mean | 0.704 ms | 0.556 ms | 0.525 ms | **−25.4%** | −5.6% |
| p50 | 0.674 ms | 0.508 ms | 0.506 ms | −24.9% | −0.4% (tie) |
| p99 | 1.008 ms | 0.953 ms | 0.799 ms | −20.7% | −16.2% |
| throughput | 1420 ops/s | 1798 ops/s | 1903 ops/s | **+34.0%** | +5.9% |
10 concurrent threads sharing one `FileSystem`:
| | master | #11369 | this PR | vs master | vs #11369 |
|---|---|---|---|---|---|
| mean | 2.324 ms | 1.785 ms | 1.730 ms | **−25.6%** | −3.1% |
| p50 | 2.240 ms | 1.701 ms | 1.648 ms | −26.4% | −3.1% |
| throughput | 4275 ops/s | 5517 ops/s | 5733 ops/s | **+34.1%** | +3.9% |
Every figure above except the single-threaded p50 has non-overlapping run
ranges between the two
optimized arms, so the margins are small but reproducible. Run-to-run spread
within an arm is under
4%.
### What the numbers do and do not support
- **Against master the win is unambiguous**: 3.00 → 1.00 RPCs per call, ~25%
lower mean and p50
latency, ~34% higher throughput, reproduced in both concurrency regimes,
the two regimes agreeing
within 0.3 pp on the mean delta.
- **Against #11369 on its best-case workload the margin is small** (3–6% on
latency and throughput,
16% on single-threaded p99) and should not be the reason to prefer this
approach. The reason is the
unconditional 1.00 and the absence of a staleness window.
- **Concurrent p99 is not resolvable at n=3** on any arm — the ranges
overlap heavily (master
2.911–5.797 ms, #11369 3.179–6.378 ms, this PR 2.697–5.489 ms). Tail
latency on a 10-thread
2000-call MiniCluster run is dominated by a handful of outliers, and it is
not what this change
targets. No p99 claim is made for the concurrent case.
- **The cold-client row (2.00 for #11369) is derived, not measured** — it
follows directly from
`2 − hitRatio` with no cache hits. The measured arm is the 90%-hit case
only.
- Latency improves less than RPC count because each call still makes one
`GetBlockChecksum` round
trip to a datanode. Two of roughly four hops are removed, and the two
removed are cheap
full-cache in-memory volume and bucket reads, which lands at about a
quarter.
Generated-by: Claude Code (claude-opus)
## What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-15951
## How was this patch tested?
New unit tests in `TestOMMetadataReader` —
`lookupFileRejectsObjectStoreLayout`,
`lookupFileAllowsLegacyLayout` — mirroring the #11226 pair.
Three tests added to `AbstractRootedOzoneFileSystemTest`, so they run under
`TestOFS`,
`TestOFSWithFSPaths` and `TestOFSWithFSO` on the existing shared cluster
rather than standing up
another one:
- `testGetFileChecksumUsesSingleOmRpc` — asserts `numBucketInfos`,
`numVolumeInfos` and
`numKeyLookups` are all unchanged across a `getFileChecksum`. The last of
these is what proves
`LookupFile` replaced `LookupKey`.
- `testGetFileChecksumRejectsObsBucket`
- `testGetFileChecksumOnDirectory`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
