[
https://issues.apache.org/jira/browse/HDDS-16952?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Andrey Yarovoy updated HDDS-16952:
----------------------------------
Description:
`BasicRootedOzoneFileSystem.delete` for a path below bucket level first calls
`getFileStatus` (one RPC since HDDS-15925) to tell a file from a directory.
What follows depends on the answer.
* *{*}File:{*}* `adapter.deleteObject(key)` → `getBucket()` (`InfoBucket`) →
`DeleteKey`. *{*}3 OM RPCs.{*}*
* *{*}Directory on FSO:{*}* `adapterImpl.getBucket()` to read the layout, then
`adapterImpl.deleteObject(path,
recursive)`, which calls `getBucket()` *{*}again{*}*, then `DeleteKey`, which
OM runs as an atomic recursive
delete. *{*}4 OM RPCs, two of them identical `InfoBucket` calls.{*}*
* *{*}Directory on LEGACY:{*}* `innerDelete` → `DeleteIterator`, which fetches
the bucket once and then lists
and batch-deletes through the bucket-taking `deleteObjects` helper.
Each link hop adds `InfoVolume` + `InfoBucket` to every `getBucket()`. `delete`
is one of the hottest file
system operations: job cleanup, staging directories, Hive overwrite.
The fetch does two jobs:
1. *{*}Layout check.{*}* On delete it is *already done* by the preceding
`getFileStatus`, against any OM:
* An OM ≥ `GET_FILE_STATUS_REJECTS_OBS` (14) rejects OBJECT_STORE server-side,
using the link-resolved layout.
* For an older OM, the adapter's `getFileStatus` falls back to `getBucket()` +
`validateBucketLayout`.
So by the time either `getBucket()` in delete runs, the bucket is known not to
be OBJECT_STORE.
2. *{*}FSO vs LEGACY.{*}* This is only needed for a directory, to choose
between OM's atomic recursive delete
and client-side iteration.
*Part 1 — no protobuf change*
* *{*}File delete: drop the `getBucket()`.{*}* After a successful
`getFileStatus` on OM ≥ 14, the layout check
is redundant. Send `DeleteKey` through a handle built locally
(`OzoneBucket.newBuilder(...)`), or through
a package-private adapter variant used only by `delete()`.
* Keep the public `deleteObject(String)` unchanged, because it can be called
without a preceding
`getFileStatus`.
* 3 → 2 RPCs. *{*}No version gate is needed{*}*, because the `getFileStatus`
that precedes it checked the
layout against an old OM too.
* *{*}FSO directory delete: reuse the fetched bucket.{*}* Pass the
`OzoneBucket` that `delete()` already
fetched into a bucket-taking `deleteObject` variant, as the existing "reduce
calls to getBucket()" helpers
do for `rename` and `deleteObjects`. 4 → 3 RPCs, with no change against any OM.
*Part 2 — opt-in field: an optimistic FSO directory delete*
The directory case still pays one `InfoBucket` to learn FSO vs LEGACY.
Following the rename Jira, add a
request field:
{code:java}
message DeleteKeyRequest {
required KeyArgs keyArgs = 1;
// Set by file system clients that rely on atomic recursive directory
// delete: OM rejects the request with NOT_SUPPORTED_OPERATION unless the
// resolved bucket layout is FILE_SYSTEM_OPTIMIZED.
optional bool requireFileSystemOptimized = 2;
}
{code}
The client sends `DeleteKey` with this field set, and with `recursive` as
requested. On
`NOT_SUPPORTED_OPERATION` it falls back to today's `getBucket()` +
`innerDelete`. FSO: 3 → 2 RPCs.
LEGACY: one extra failed RPC on a path that already lists and batch-deletes.
The rejection has to be opt-in because `DeleteKey` is dual-use.
`OMKeyDeleteRequest` is registered for
`BucketLayout.OBJECT_STORE` (that registration also serves LEGACY), and these
callers send it on any
layout:
* the S3 Gateway's `DeleteObject`;
* `cli-shell` (`DeleteKeyHandler`);
* `freon`;
* `TrashOzoneFileSystem`;
* the public `OzoneBucket.deleteKey` / `deleteDirectory`.
None of them set the field.
*OM.* Check on the leader before Ratis, in `preExecute` or right after
`createClientRequest`. Use the
link-resolved layout `BucketLayoutAwareOMKeyRequestFactory` already read.
Neither delete class uses
`NOT_SUPPORTED_OPERATION` today. Do not put the check in `createRequest`
itself, because that also runs
at apply time.
*Client.* Use the new path when `proxy.getOmVersion() >=` a new
`OzoneManagerVersion` constant. Place it
after `LIST_STATUS_REJECTS_OBS(101)`, never below `ZDU(100)`; the number is
assigned at merge. Against any
older OM, including one that reports ZDU, use the Part 1 path.
*{*}Watch the error handling:{*}* `adapterImpl.deleteObject` catches *every*
`OMException` other than
`DIRECTORY_NOT_EMPTY` / `INVALID_KEY_NAME` and returns `false`. Today the
OBJECT_STORE rejection escapes
that catch only because it is an unchecked `IllegalArgumentException`. The new
`NOT_SUPPORTED_OPERATION` must be handled before the generic catch. Otherwise a
LEGACY directory delete
would silently return `false` and never fall back.
`DeleteKeys` (batch) is *{*}out of scope{*}*. Its file system callers already
pass a fetched bucket
(`DeleteIterator`), or sit on the cold recursive-bucket-delete path
(`DeleteIteratorWithFSO`, one
`InfoBucket` per batch through the public `deleteObjects(List)`). A Part
1-style pass of the iterator's
bucket into the helper fixes the latter without any field. The S3 Gateway's
`DeleteObjects` and
`KeyLifecycleService` use it on all layouts.
*Compatibility*
| |file|FSO dir|LEGACY dir|
|—|—|—|—|
|Today|3|4|2 + iteration|
|Part 1 (any OM)|2|3|2 + iteration|
|Part 2 (new OM)|2|2|3 + iteration|
Old clients, the S3 Gateway, the shell, trash and direct `OzoneBucket` callers
send no field, so their
behavior is unchanged. Part 2 adds one optional proto field and no result code
or configuration.
*{*}Recommendation:{*}* file Part 1 first. It is client-only, has no
compatibility surface, and removes 1–2
RPCs from every delete. Part 2 should follow the rename Jira and use the same
field semantics.
*Testing*
* `BasicRootedOzoneFileSystem` / adapter (Mockito):
* File delete: `getFileStatus` + `DeleteKey`, and no `getBucket()`.
* FSO directory: exactly one `getBucket()` (Part 1), then none (Part 2).
* Part 2 `NOT_SUPPORTED_OPERATION` on LEGACY falls back to `innerDelete` and
does *{*}not{*}* return `false`.
*`DIRECTORY_NOT_EMPTY` → `PathIsNotEmptyDirectoryException`, as today.
* Older OMs, including one at `ZDU`, fall back.
* OM request tests (Part 2):
* With the field set, FSO succeeds and LEGACY / OBJECT_STORE are rejected
before Ratis.
* Without the field, everything succeeds. This guards S3 `DeleteObject`, the
shell, trash and lifecycle.
* `AbstractRootedOzoneFileSystemTest` (`TestOFS`, `TestOFSWithFSO`): the
existing delete suite passes on
both layouts, including orphan-link and bucket deletes. Count RPCs in the
adapter test, not with
OMMetrics deltas (HDDS-16513).
* Benchmark `TestOfsDeleteRpcBenchmark`, same shape as
`TestOfsListStatusRpcBenchmark`:
* Two cases: a file delete, and a non-recursive empty-directory delete on FSO.
* Single-threaded and 10 threads, n=3 per arm, built and run strictly one
after the other.
* Invariants: file 3 → 2, FSO directory 4 → 3 (Part 1) → 2 (Part 2).
was:
`BasicRootedOzoneFileSystem.delete` for a path below bucket level first calls
`getFileStatus` (one RPC since HDDS-15925) to tell a file from a directory.
What follows depends on the answer.
* **File:** `adapter.deleteObject(key)` → `getBucket()` (`InfoBucket`) →
`DeleteKey`. **3 OM RPCs.**
* **Directory on FSO:** `adapterImpl.getBucket()` to read the layout, then
`adapterImpl.deleteObject(path,
recursive)`, which calls `getBucket()` **again**, then `DeleteKey`, which OM
runs as an atomic recursive
delete. **4 OM RPCs, two of them identical `InfoBucket` calls.**
* **Directory on LEGACY:** `innerDelete` → `DeleteIterator`, which fetches the
bucket once and then lists
and batch-deletes through the bucket-taking `deleteObjects` helper.
Each link hop adds `InfoVolume` + `InfoBucket` to every `getBucket()`. `delete`
is one of the hottest file
system operations: job cleanup, staging directories, Hive overwrite.
The fetch does two jobs:
1. **Layout check.** On delete it is *already done* by the preceding
`getFileStatus`, against any OM:
* An OM ≥ `GET_FILE_STATUS_REJECTS_OBS` (14) rejects OBJECT_STORE server-side,
using the link-resolved layout.
* For an older OM, the adapter's `getFileStatus` falls back to `getBucket()` +
`validateBucketLayout`.
So by the time either `getBucket()` in delete runs, the bucket is known not to
be OBJECT_STORE.
2. **FSO vs LEGACY.** This is only needed for a directory, to choose between
OM's atomic recursive delete
and client-side iteration.
### Part 1 — no protobuf change
* **File delete: drop the `getBucket()`.** After a successful `getFileStatus`
on OM ≥ 14, the layout check
is redundant. Send `DeleteKey` through a handle built locally
(`OzoneBucket.newBuilder(...)`), or through
a package-private adapter variant used only by `delete()`.
* Keep the public `deleteObject(String)` unchanged, because it can be called
without a preceding
`getFileStatus`.
* 3 → 2 RPCs. **No version gate is needed**, because the `getFileStatus` that
precedes it checked the
layout against an old OM too.
* **FSO directory delete: reuse the fetched bucket.** Pass the `OzoneBucket`
that `delete()` already
fetched into a bucket-taking `deleteObject` variant, as the existing "reduce
calls to getBucket()" helpers
do for `rename` and `deleteObjects`. 4 → 3 RPCs, with no change against any OM.
### Part 2 — opt-in field: an optimistic FSO directory delete
The directory case still pays one `InfoBucket` to learn FSO vs LEGACY.
Following the rename Jira, add a
request field:
{code}
message DeleteKeyRequest {
required KeyArgs keyArgs = 1;
// Set by file system clients that rely on atomic recursive directory
// delete: OM rejects the request with NOT_SUPPORTED_OPERATION unless the
// resolved bucket layout is FILE_SYSTEM_OPTIMIZED.
optional bool requireFileSystemOptimized = 2;
}
{code}
The client sends `DeleteKey` with this field set, and with `recursive` as
requested. On
`NOT_SUPPORTED_OPERATION` it falls back to today's `getBucket()` +
`innerDelete`. FSO: 3 → 2 RPCs.
LEGACY: one extra failed RPC on a path that already lists and batch-deletes.
The rejection has to be opt-in because `DeleteKey` is dual-use.
`OMKeyDeleteRequest` is registered for
`BucketLayout.OBJECT_STORE` (that registration also serves LEGACY), and these
callers send it on any
layout:
* the S3 Gateway's `DeleteObject`;
* `cli-shell` (`DeleteKeyHandler`);
* `freon`;
* `TrashOzoneFileSystem`;
* the public `OzoneBucket.deleteKey` / `deleteDirectory`.
None of them set the field.
*OM.* Check on the leader before Ratis, in `preExecute` or right after
`createClientRequest`. Use the
link-resolved layout `BucketLayoutAwareOMKeyRequestFactory` already read.
Neither delete class uses
`NOT_SUPPORTED_OPERATION` today. Do not put the check in `createRequest`
itself, because that also runs
at apply time.
*Client.* Use the new path when `proxy.getOmVersion() >=` a new
`OzoneManagerVersion` constant. Place it
after `LIST_STATUS_REJECTS_OBS(101)`, never below `ZDU(100)`; the number is
assigned at merge. Against any
older OM, including one that reports ZDU, use the Part 1 path.
**Watch the error handling:** `adapterImpl.deleteObject` catches *every*
`OMException` other than
`DIRECTORY_NOT_EMPTY` / `INVALID_KEY_NAME` and returns `false`. Today the
OBJECT_STORE rejection escapes
that catch only because it is an unchecked `IllegalArgumentException`. The new
`NOT_SUPPORTED_OPERATION` must be handled before the generic catch. Otherwise a
LEGACY directory delete
would silently return `false` and never fall back.
`DeleteKeys` (batch) is **out of scope**. Its file system callers already pass
a fetched bucket
(`DeleteIterator`), or sit on the cold recursive-bucket-delete path
(`DeleteIteratorWithFSO`, one
`InfoBucket` per batch through the public `deleteObjects(List)`). A Part
1-style pass of the iterator's
bucket into the helper fixes the latter without any field. The S3 Gateway's
`DeleteObjects` and
`KeyLifecycleService` use it on all layouts.
### Compatibility
| | file | FSO dir | LEGACY dir |
|---|---|---|---|
| Today | 3 | 4 | 2 + iteration |
| Part 1 (any OM) | 2 | 3 | 2 + iteration |
| Part 2 (new OM) | 2 | 2 | 3 + iteration |
Old clients, the S3 Gateway, the shell, trash and direct `OzoneBucket` callers
send no field, so their
behavior is unchanged. Part 2 adds one optional proto field and no result code
or configuration.
**Recommendation:** file Part 1 first. It is client-only, has no compatibility
surface, and removes 1–2
RPCs from every delete. Part 2 should follow the rename Jira and use the same
field semantics.
Points for review:
* The BUCKET/READ ACL check from `getBucketDetails` is dropped. `DeleteKey`
keeps its own key/bucket ACL
checks.
* There is a race in Part 1: a bucket deleted and recreated as OBJECT_STORE
between `getFileStatus` and
`DeleteKey` would let one key delete through. That is the same outcome as an
object-store client
deleting that key, so no file system invariant is at stake.
## Testing
* `BasicRootedOzoneFileSystem` / adapter (Mockito):
* File delete: `getFileStatus` + `DeleteKey`, and no `getBucket()`.
* FSO directory: exactly one `getBucket()` (Part 1), then none (Part 2).
* Part 2 `NOT_SUPPORTED_OPERATION` on LEGACY falls back to `innerDelete` and
does **not** return `false`.
*`DIRECTORY_NOT_EMPTY` → `PathIsNotEmptyDirectoryException`, as today.
* Older OMs, including one at `ZDU`, fall back.
* OM request tests (Part 2):
* With the field set, FSO succeeds and LEGACY / OBJECT_STORE are rejected
before Ratis.
* Without the field, everything succeeds. This guards S3 `DeleteObject`, the
shell, trash and lifecycle.
* `AbstractRootedOzoneFileSystemTest` (`TestOFS`, `TestOFSWithFSO`): the
existing delete suite passes on
both layouts, including orphan-link and bucket deletes. Count RPCs in the
adapter test, not with
OMMetrics deltas (HDDS-16513).
* Benchmark `TestOfsDeleteRpcBenchmark`, same shape as
`TestOfsListStatusRpcBenchmark`:
* Two cases: a file delete, and a non-recursive empty-directory delete on FSO.
* Single-threaded and 10 threads, n=3 per arm, built and run strictly one after
the other.
* Invariants: file 3 → 2, FSO directory 4 → 3 (Part 1) → 2 (Part 2).
> Remove the per-call InfoBucket calls from rooted-ofs delete
> -----------------------------------------------------------
>
> Key: HDDS-16952
> URL: https://issues.apache.org/jira/browse/HDDS-16952
> Project: Apache Ozone
> Issue Type: Improvement
> Components: Ozone Client
> Reporter: Andrey Yarovoy
> Assignee: Andrey Yarovoy
> Priority: Major
>
> `BasicRootedOzoneFileSystem.delete` for a path below bucket level first calls
> `getFileStatus` (one RPC since HDDS-15925) to tell a file from a directory.
> What follows depends on the answer.
> * *{*}File:{*}* `adapter.deleteObject(key)` → `getBucket()` (`InfoBucket`) →
> `DeleteKey`. *{*}3 OM RPCs.{*}*
> * *{*}Directory on FSO:{*}* `adapterImpl.getBucket()` to read the layout,
> then `adapterImpl.deleteObject(path,
> recursive)`, which calls `getBucket()` *{*}again{*}*, then `DeleteKey`, which
> OM runs as an atomic recursive
> delete. *{*}4 OM RPCs, two of them identical `InfoBucket` calls.{*}*
> * *{*}Directory on LEGACY:{*}* `innerDelete` → `DeleteIterator`, which
> fetches the bucket once and then lists
> and batch-deletes through the bucket-taking `deleteObjects` helper.
> Each link hop adds `InfoVolume` + `InfoBucket` to every `getBucket()`.
> `delete` is one of the hottest file
> system operations: job cleanup, staging directories, Hive overwrite.
> The fetch does two jobs:
> 1. *{*}Layout check.{*}* On delete it is *already done* by the preceding
> `getFileStatus`, against any OM:
> * An OM ≥ `GET_FILE_STATUS_REJECTS_OBS` (14) rejects OBJECT_STORE
> server-side, using the link-resolved layout.
> * For an older OM, the adapter's `getFileStatus` falls back to `getBucket()`
> + `validateBucketLayout`.
> So by the time either `getBucket()` in delete runs, the bucket is known not
> to be OBJECT_STORE.
> 2. *{*}FSO vs LEGACY.{*}* This is only needed for a directory, to choose
> between OM's atomic recursive delete
> and client-side iteration.
> *Part 1 — no protobuf change*
> * *{*}File delete: drop the `getBucket()`.{*}* After a successful
> `getFileStatus` on OM ≥ 14, the layout check
> is redundant. Send `DeleteKey` through a handle built locally
> (`OzoneBucket.newBuilder(...)`), or through
> a package-private adapter variant used only by `delete()`.
> * Keep the public `deleteObject(String)` unchanged, because it can be called
> without a preceding
> `getFileStatus`.
> * 3 → 2 RPCs. *{*}No version gate is needed{*}*, because the `getFileStatus`
> that precedes it checked the
> layout against an old OM too.
> * *{*}FSO directory delete: reuse the fetched bucket.{*}* Pass the
> `OzoneBucket` that `delete()` already
> fetched into a bucket-taking `deleteObject` variant, as the existing "reduce
> calls to getBucket()" helpers
> do for `rename` and `deleteObjects`. 4 → 3 RPCs, with no change against any
> OM.
> *Part 2 — opt-in field: an optimistic FSO directory delete*
> The directory case still pays one `InfoBucket` to learn FSO vs LEGACY.
> Following the rename Jira, add a
> request field:
> {code:java}
> message DeleteKeyRequest {
> required KeyArgs keyArgs = 1;
> // Set by file system clients that rely on atomic recursive directory
> // delete: OM rejects the request with NOT_SUPPORTED_OPERATION unless the
> // resolved bucket layout is FILE_SYSTEM_OPTIMIZED.
> optional bool requireFileSystemOptimized = 2;
> }
> {code}
> The client sends `DeleteKey` with this field set, and with `recursive` as
> requested. On
> `NOT_SUPPORTED_OPERATION` it falls back to today's `getBucket()` +
> `innerDelete`. FSO: 3 → 2 RPCs.
> LEGACY: one extra failed RPC on a path that already lists and batch-deletes.
> The rejection has to be opt-in because `DeleteKey` is dual-use.
> `OMKeyDeleteRequest` is registered for
> `BucketLayout.OBJECT_STORE` (that registration also serves LEGACY), and these
> callers send it on any
> layout:
> * the S3 Gateway's `DeleteObject`;
> * `cli-shell` (`DeleteKeyHandler`);
> * `freon`;
> * `TrashOzoneFileSystem`;
> * the public `OzoneBucket.deleteKey` / `deleteDirectory`.
> None of them set the field.
> *OM.* Check on the leader before Ratis, in `preExecute` or right after
> `createClientRequest`. Use the
> link-resolved layout `BucketLayoutAwareOMKeyRequestFactory` already read.
> Neither delete class uses
> `NOT_SUPPORTED_OPERATION` today. Do not put the check in `createRequest`
> itself, because that also runs
> at apply time.
> *Client.* Use the new path when `proxy.getOmVersion() >=` a new
> `OzoneManagerVersion` constant. Place it
> after `LIST_STATUS_REJECTS_OBS(101)`, never below `ZDU(100)`; the number is
> assigned at merge. Against any
> older OM, including one that reports ZDU, use the Part 1 path.
> *{*}Watch the error handling:{*}* `adapterImpl.deleteObject` catches *every*
> `OMException` other than
> `DIRECTORY_NOT_EMPTY` / `INVALID_KEY_NAME` and returns `false`. Today the
> OBJECT_STORE rejection escapes
> that catch only because it is an unchecked `IllegalArgumentException`. The new
> `NOT_SUPPORTED_OPERATION` must be handled before the generic catch. Otherwise
> a LEGACY directory delete
> would silently return `false` and never fall back.
> `DeleteKeys` (batch) is *{*}out of scope{*}*. Its file system callers already
> pass a fetched bucket
> (`DeleteIterator`), or sit on the cold recursive-bucket-delete path
> (`DeleteIteratorWithFSO`, one
> `InfoBucket` per batch through the public `deleteObjects(List)`). A Part
> 1-style pass of the iterator's
> bucket into the helper fixes the latter without any field. The S3 Gateway's
> `DeleteObjects` and
> `KeyLifecycleService` use it on all layouts.
> *Compatibility*
> | |file|FSO dir|LEGACY dir|
> |—|—|—|—|
> |Today|3|4|2 + iteration|
> |Part 1 (any OM)|2|3|2 + iteration|
> |Part 2 (new OM)|2|2|3 + iteration|
> Old clients, the S3 Gateway, the shell, trash and direct `OzoneBucket`
> callers send no field, so their
> behavior is unchanged. Part 2 adds one optional proto field and no result
> code or configuration.
> *{*}Recommendation:{*}* file Part 1 first. It is client-only, has no
> compatibility surface, and removes 1–2
> RPCs from every delete. Part 2 should follow the rename Jira and use the same
> field semantics.
> *Testing*
> * `BasicRootedOzoneFileSystem` / adapter (Mockito):
> * File delete: `getFileStatus` + `DeleteKey`, and no `getBucket()`.
> * FSO directory: exactly one `getBucket()` (Part 1), then none (Part 2).
> * Part 2 `NOT_SUPPORTED_OPERATION` on LEGACY falls back to `innerDelete` and
> does *{*}not{*}* return `false`.
> *`DIRECTORY_NOT_EMPTY` → `PathIsNotEmptyDirectoryException`, as today.
> * Older OMs, including one at `ZDU`, fall back.
> * OM request tests (Part 2):
> * With the field set, FSO succeeds and LEGACY / OBJECT_STORE are rejected
> before Ratis.
> * Without the field, everything succeeds. This guards S3 `DeleteObject`, the
> shell, trash and lifecycle.
> * `AbstractRootedOzoneFileSystemTest` (`TestOFS`, `TestOFSWithFSO`): the
> existing delete suite passes on
> both layouts, including orphan-link and bucket deletes. Count RPCs in the
> adapter test, not with
> OMMetrics deltas (HDDS-16513).
> * Benchmark `TestOfsDeleteRpcBenchmark`, same shape as
> `TestOfsListStatusRpcBenchmark`:
> * Two cases: a file delete, and a non-recursive empty-directory delete on
> FSO.
> * Single-threaded and 10 threads, n=3 per arm, built and run strictly one
> after the other.
> * Invariants: file 3 → 2, FSO directory 4 → 3 (Part 1) → 2 (Part 2).
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
