Andrey Yarovoy created HDDS-16952:
-------------------------------------
Summary: 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
`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).
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
