Andrey Yarovoy created HDDS-16947:
-------------------------------------
Summary: Remove the per-call InfoBucket from rooted-ofs setTimes
Key: HDDS-16947
URL: https://issues.apache.org/jira/browse/HDDS-16947
Project: Apache Ozone
Issue Type: Improvement
Components: Ozone Client
Reporter: Andrey Yarovoy
`BasicRootedOzoneClientAdapterImpl.setTimes` calls `getBucket()` (`InfoBucket`,
plus `InfoVolume` +
`InfoBucket` per link hop) before `OzoneBucket.setTimes`. So every
`FileSystem.setTimes` sends at least 2 OM
requests. The fetch does one job: `OzoneFSUtils.validateBucketLayout`, which
rejects OBJECT_STORE. Nothing
else is read from the bucket. `setTimes` is called once per file by tools that
preserve times, for
example `distcp -pt`, and through HttpFS.
OM does not check the layout today. `OMKeySetTimesRequest` is registered for
`BucketLayout.OBJECT_STORE`
in `BucketLayoutAwareOMKeyRequestFactory` (that registration serves LEGACY as
well), so `SetTimes`
succeeds on an OBJECT_STORE bucket.
### Why the rejection is opt-in
No in-tree object store caller sends `SetTimes`: nothing in `s3gateway`,
`cli-shell`, `freon` or `recon`.
The only callers are the o3fs and ofs file systems, and HttpFS through them.
But `OzoneBucket.setTimes` is
public Java API, and it works on OBJECT_STORE buckets today. An unconditional
rejection would break that.
The client asks for the check:
{code}
message SetTimesRequest {
required KeyArgs keyArgs = 1;
required uint64 mtime = 2;
required uint64 atime = 3;
// Set by file system clients: OM rejects the request with
// NOT_SUPPORTED_OPERATION if the resolved bucket layout is OBJECT_STORE.
optional bool requireFileSystemLayout = 4;
}
{code}
### Proposed change
*OM.* Check before Ratis, on the leader. Do it in
`OMKeySetTimesRequest.preExecute`, or right after
`createClientRequest` in `OMExecutionFlow.submitExecutionToRatis`. Use the
link-resolved layout that
`BucketLayoutAwareOMKeyRequestFactory.createRequest` has just read through
`OzoneManagerUtils.getBucketLayout`, so no new lookup is added. Reuse
`OzoneFSUtils.validateBucketLayout` for the message and wrap it in
`OMException(NOT_SUPPORTED_OPERATION)`, as
`OmMetadataReader.validateFileSystemLayout` does.
Do not put the check inside `createRequest` itself, because that also runs at
apply time on every OM
(`OzoneManagerRequestHandler.handleWriteRequestImpl`). A check there would be
deterministic: the client
only sets the field once the OM reports the new version, which happens after
finalization. But a
rejection at apply still costs a Ratis log entry, and a pre-Ratis rejection
costs nothing. Note:
`OMKeySetTimesRequest.preExecute` rebuilds `KeyArgs` with
`KeyArgs.newBuilder()`, so it does not carry
the field forward. That is fine if the check runs before the rebuild.
*Client.*
* `OmKeyArgs` already has `requireFileSystemLayout` (HDDS-16760).
`OzoneManagerProtocolClientSideTranslatorPB.setTimes`
builds `KeyArgs` field by field, so add the `SetTimesRequest` field there.
* `ClientProtocol` / `OzoneBucket` need a way for the adapter to set it. Mirror
what HDDS-16760 did with
`ListStatusLightOptions`, or add an overload that takes `OmKeyArgs`.
* The adapter skips `getBucket()` when `proxy.getOmVersion() >=` a new
`OzoneManagerVersion` constant.
Place the constant after `LIST_STATUS_REJECTS_OBS(101)`; the number is assigned
at merge, so
coordinate it with sibling Jiras. Never put it below `ZDU(100)`, because an OM
without this change can
report ZDU once finalized. It then ignores the field.
* Map `NOT_SUPPORTED_OPERATION` to `IllegalArgumentException`.
* Against any older OM, including one that reports ZDU, keep the current
`getBucket()` path.
### Compatibility
| | Behavior |
|---|---|
| New client, new OM | 1 RPC; OM rejects OBJECT_STORE |
| New client, older OM (incl. ZDU) | `getBucket()` + `SetTimes`, as today |
| Old client / direct `OzoneBucket.setTimes`, new OM | field absent ⇒
unchanged, OBJECT_STORE still accepted |
There is one new optional proto field, no new result code and no new
configuration. `proto.lock` is not
updated (HDDS-14779 / HDDS-16760 precedent).
Points for review: the BUCKET/READ ACL check from `getBucketDetails` is
dropped. `OMKeySetTimesRequest`
keeps its own key ACL check, and OM checks READ on each link it follows.
## Testing
* Adapter (Mockito):
* exactly one OM call, carrying the field;
*`NOT_SUPPORTED_OPERATION` → `IllegalArgumentException`;
* an older OM, including `ZDU`, falls back to `getBucket()`.
* OM request test, for each layout:
* with the field set, OBJECT_STORE is rejected before Ratis, while FSO and
**LEGACY** still succeed.
LEGACY guards against the `getKey(Type, BucketLayout)` suffix subtlety:
OBJECT_STORE-registered
classes also serve LEGACY, so test the layout value, never a missing map entry;
* without the field, OBJECT_STORE still succeeds;
* a link to an OBJECT_STORE bucket is rejected, which proves the resolved
layout is used.
* Translator/handler: the field round-trips.
* `AbstractRootedOzoneFileSystemTest`: `setTimes` on an OBJECT_STORE bucket
fails with
`IllegalArgumentException`. Keep RPC counting in the adapter test, not on
OMMetrics deltas (HDDS-16513).
* Benchmark: `TestOfsSetTimesRpcBenchmark`, same shape as
`TestOfsListStatusRpcBenchmark`:
* 200 FSO buckets, shuffled accesses, single-threaded and 10 threads;
* baseline vs change, n=3 per arm, built and run strictly one after the other;
* invariant on the new arm: exactly 1 OM RPC per `setTimes`, from 2 before.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
