[
https://issues.apache.org/jira/browse/HDDS-16947?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Andrey Yarovoy reassigned HDDS-16947:
-------------------------------------
Assignee: Andrey Yarovoy
> 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
> Assignee: Andrey Yarovoy
> Priority: Major
>
> `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]
