[
https://issues.apache.org/jira/browse/HDDS-16946?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Andrey Yarovoy updated HDDS-16946:
----------------------------------
Description:
`BasicRootedOzoneClientAdapterImpl.isFileClosed` and `recoverFilePrepare` call
`getBucket()` before their real RPC. That is an `InfoBucket` (plus `InfoVolume`
+ `InfoBucket` per link hop). The fetch does one job:
`OzoneFSUtils.validateBucketLayout`, which throws `IllegalArgumentException`
for OBJECT_STORE. Neither
site reads anything else from the returned `OzoneBucket`, apart from volume and
bucket names it already
has. These are the hsync paths: HBase calls `isFileClosed` while it waits for
lease recovery, and `recoverLease` on region server failover.
* {*}{{*}}isFileClosed{{*}}{*} issues `GetFileStatus`
(`OzoneBucket.getFileStatus` → `ClientProtocol.getOzoneFileStatus`,
headOp=false). `OmMetadataReader.getFileStatus` has rejected OBJECT_STORE with
`NOT_SUPPORTED_OPERATION`
unconditionally since HDDS-15925
(`OzoneManagerVersion.GET_FILE_STATUS_REJECTS_OBS`, 14). There, the
layout comes from `ResolvedBucket`, so links are resolved.
* {*}{{*}}recoverLease{{*}}{*} issues `RecoverLease`.
`OzoneManagerRatisUtils.createClientRequest` looks up the
link-resolved layout and rejects every non-FSO bucket (LEGACY too) before
Ratis, with
"Bucket ... is not FSO layout. It does not support lease recovery". This check
dates from HDDS-7769
(2023). `RpcClient.recoverLease` refuses to send the RPC at all unless the OM
is at least
`HBASE_SUPPORT` (7), which was added in 2024. So every OM that can receive this
RPC already has the check.
#
##
### Proposed change
*isFileClosed.* When `proxy.getOmVersion() >= GET_FILE_STATUS_REJECTS_OBS`, call
`proxy.getOzoneFileStatus(volume, bucket, key, false)` directly, the same way
HDDS-15925 changed
`getFileStatus`. Map `NOT_SUPPORTED_OPERATION` to `IllegalArgumentException`.
Against an older OM keep the
current `getBucket()` path.
Keep the `isSnapshotPath()` check before the RPC. Today it runs after
`getBucket()`, so a snapshot path in
a missing bucket reports `FileNotFoundException`, and after the change it
reports "file is in a
snapshot". Either keep that order (cheap: the snapshot path check is local) or
call the difference out
in the PR.
*recoverLease.* Drop the `getBucket()` and pass `ofsPath.getVolumeName()` /
`getBucketName()`. Those are
the names `bucket.getVolumeName()` / `getName()` return today, because the link
handle carries the
link's own names.
The one behaviour change is the exception type for an OBJECT_STORE bucket.
Today the client throws
`IllegalArgumentException` before any RPC. Without the fetch, OM's plain
`IOException` reaches the
client as `OMException(INTERNAL_ERROR)`. To keep the exception type unchanged,
follow the HDDS-16760
pattern:
* OM throws `OMException(NOT_SUPPORTED_OPERATION)` instead of `IOException`,
with the same message.
* Add a new `OzoneManagerVersion` constant, placed after
`LIST_STATUS_REJECTS_OBS(101)`. The number is
assigned at merge; coordinate it with sibling Jiras.
* At or above that version the client skips `getBucket()` and maps
`NOT_SUPPORTED_OPERATION` to
`IllegalArgumentException`. Below it the client keeps `getBucket()`.
LEGACY buckets behave as before: the client never rejected them, and OM rejects
them under either code.
*Compatibility*
| |isFileClosed|recoverLease|
|—|—|—|
|New client, OM ≥ new constant|1 RPC, OBS → `IllegalArgumentException`|1 RPC,
OBS → `IllegalArgumentException`|
|New client, older OM|1 RPC for OM ≥ 14; `getBucket()` path below|`getBucket()`
path, unchanged|
|Old client, new OM|unchanged|unchanged, except LEGACY now gets
`NOT_SUPPORTED_OPERATION` instead of `INTERNAL_ERROR`|
There is no protobuf change and no new configuration.
*Testing*
* Adapter (Mockito partial mock, like
`TestBasicRootedOzoneClientAdapterHeadOp`):
* Both operations issue exactly one OM call and no `getBucket()`.
*`NOT_SUPPORTED_OPERATION` → `IllegalArgumentException`.
*`FILE_NOT_FOUND` / `KEY_NOT_FOUND` / `BUCKET_NOT_FOUND` keep today's
`FileNotFoundException` mapping.
* An older OM falls back to `getBucket()`, including an OM that reports `ZDU`
for recoverLease.
* OM: `TestOMRecoverLeaseRequest` (or the `createClientRequest` test) asserts
`NOT_SUPPORTED_OPERATION`
for OBJECT_STORE and LEGACY, and success for FSO.
* `AbstractRootedOzoneFileSystemTest`: `isFileClosed` / `recoverLease` on an
OBJECT_STORE bucket still fail
with `IllegalArgumentException`. Assert the RPC-count property in the adapter
test, not by OMMetrics
deltas. OMMetrics is cluster-global and the trash emptier moves it (HDDS-16513).
* Benchmark: the cost of these calls is too low and they are too rare to be
worth a dedicated
benchmark. The RPC count goes from 2 to 1 per call, and the adapter test
asserts it exactly.
was:
`BasicRootedOzoneClientAdapterImpl.isFileClosed` and `recoverFilePrepare` call
`getBucket()` before their real RPC. That is an `InfoBucket` (plus `InfoVolume`
+ `InfoBucket` per link hop). The fetch does one job:
`OzoneFSUtils.validateBucketLayout`, which throws `IllegalArgumentException`
for OBJECT_STORE. Neither
site reads anything else from the returned `OzoneBucket`, apart from volume and
bucket names it already
has. These are the hsync paths: HBase calls `isFileClosed` while it waits for
lease recovery, and `recoverLease` on region server failover.
* *{*}isFileClosed{*}* issues `GetFileStatus` (`OzoneBucket.getFileStatus` →
`ClientProtocol.getOzoneFileStatus`,
headOp=false). `OmMetadataReader.getFileStatus` has rejected OBJECT_STORE with
`NOT_SUPPORTED_OPERATION`
unconditionally since HDDS-15925
(`OzoneManagerVersion.GET_FILE_STATUS_REJECTS_OBS`, 14). There, the
layout comes from `ResolvedBucket`, so links are resolved.
* *{*}recoverLease{*}* issues `RecoverLease`.
`OzoneManagerRatisUtils.createClientRequest` looks up the
link-resolved layout and rejects every non-FSO bucket (LEGACY too) before
Ratis, with
"Bucket ... is not FSO layout. It does not support lease recovery". This check
dates from HDDS-7769
(2023). `RpcClient.recoverLease` refuses to send the RPC at all unless the OM
is at least
`HBASE_SUPPORT` (7), which was added in 2024. So every OM that can receive this
RPC already has the check.
#
##
### Proposed change
*isFileClosed.* When `proxy.getOmVersion() >= GET_FILE_STATUS_REJECTS_OBS`, call
`proxy.getOzoneFileStatus(volume, bucket, key, false)` directly, the same way
HDDS-15925 changed
`getFileStatus`. Map `NOT_SUPPORTED_OPERATION` to `IllegalArgumentException`.
Against an older OM keep the
current `getBucket()` path.
Keep the `isSnapshotPath()` check before the RPC. Today it runs after
`getBucket()`, so a snapshot path in
a missing bucket reports `FileNotFoundException`, and after the change it
reports "file is in a
snapshot". Either keep that order (cheap: the snapshot path check is local) or
call the difference out
in the PR.
*recoverLease.* Drop the `getBucket()` and pass `ofsPath.getVolumeName()` /
`getBucketName()`. Those are
the names `bucket.getVolumeName()` / `getName()` return today, because the link
handle carries the
link's own names.
The one behaviour change is the exception type for an OBJECT_STORE bucket.
Today the client throws
`IllegalArgumentException` before any RPC. Without the fetch, OM's plain
`IOException` reaches the
client as `OMException(INTERNAL_ERROR)`. To keep the exception type unchanged,
follow the HDDS-16760
pattern:
* OM throws `OMException(NOT_SUPPORTED_OPERATION)` instead of `IOException`,
with the same message.
* Add a new `OzoneManagerVersion` constant, placed after
`LIST_STATUS_REJECTS_OBS(101)`. The number is
assigned at merge; coordinate it with sibling Jiras.
* At or above that version the client skips `getBucket()` and maps
`NOT_SUPPORTED_OPERATION` to
`IllegalArgumentException`. Below it the client keeps `getBucket()`.
LEGACY buckets behave as before: the client never rejected them, and OM rejects
them under either code.
#
##
### Compatibility
| |isFileClosed|recoverLease|
|—|—|—|
|New client, OM ≥ new constant|1 RPC, OBS → `IllegalArgumentException`|1 RPC,
OBS → `IllegalArgumentException`|
|New client, older OM|1 RPC for OM ≥ 14; `getBucket()` path below|`getBucket()`
path, unchanged|
|Old client, new OM|unchanged|unchanged, except LEGACY now gets
`NOT_SUPPORTED_OPERATION` instead of `INTERNAL_ERROR`|
There is no protobuf change and no new configuration.
*Testing*
* Adapter (Mockito partial mock, like
`TestBasicRootedOzoneClientAdapterHeadOp`):
* Both operations issue exactly one OM call and no `getBucket()`.
*`NOT_SUPPORTED_OPERATION` → `IllegalArgumentException`.
*`FILE_NOT_FOUND` / `KEY_NOT_FOUND` / `BUCKET_NOT_FOUND` keep today's
`FileNotFoundException` mapping.
* An older OM falls back to `getBucket()`, including an OM that reports `ZDU`
for recoverLease.
* OM: `TestOMRecoverLeaseRequest` (or the `createClientRequest` test) asserts
`NOT_SUPPORTED_OPERATION`
for OBJECT_STORE and LEGACY, and success for FSO.
* `AbstractRootedOzoneFileSystemTest`: `isFileClosed` / `recoverLease` on an
OBJECT_STORE bucket still fail
with `IllegalArgumentException`. Assert the RPC-count property in the adapter
test, not by OMMetrics
deltas. OMMetrics is cluster-global and the trash emptier moves it (HDDS-16513).
* Benchmark: the cost of these calls is too low and they are too rare to be
worth a dedicated
benchmark. The RPC count goes from 2 to 1 per call, and the adapter test
asserts it exactly.
> Remove the per-call InfoBucket from rooted-ofs isFileClosed and recoverLease
> ----------------------------------------------------------------------------
>
> Key: HDDS-16946
> URL: https://issues.apache.org/jira/browse/HDDS-16946
> Project: Apache Ozone
> Issue Type: Improvement
> Components: OM
> Reporter: Andrey Yarovoy
> Assignee: Andrey Yarovoy
> Priority: Major
>
> `BasicRootedOzoneClientAdapterImpl.isFileClosed` and `recoverFilePrepare`
> call `getBucket()` before their real RPC. That is an `InfoBucket` (plus
> `InfoVolume` + `InfoBucket` per link hop). The fetch does one job:
> `OzoneFSUtils.validateBucketLayout`, which throws `IllegalArgumentException`
> for OBJECT_STORE. Neither
> site reads anything else from the returned `OzoneBucket`, apart from volume
> and bucket names it already
> has. These are the hsync paths: HBase calls `isFileClosed` while it waits for
> lease recovery, and `recoverLease` on region server failover.
> * {*}{{*}}isFileClosed{{*}}{*} issues `GetFileStatus`
> (`OzoneBucket.getFileStatus` → `ClientProtocol.getOzoneFileStatus`,
> headOp=false). `OmMetadataReader.getFileStatus` has rejected OBJECT_STORE
> with `NOT_SUPPORTED_OPERATION`
> unconditionally since HDDS-15925
> (`OzoneManagerVersion.GET_FILE_STATUS_REJECTS_OBS`, 14). There, the
> layout comes from `ResolvedBucket`, so links are resolved.
> * {*}{{*}}recoverLease{{*}}{*} issues `RecoverLease`.
> `OzoneManagerRatisUtils.createClientRequest` looks up the
> link-resolved layout and rejects every non-FSO bucket (LEGACY too) before
> Ratis, with
> "Bucket ... is not FSO layout. It does not support lease recovery". This
> check dates from HDDS-7769
> (2023). `RpcClient.recoverLease` refuses to send the RPC at all unless the OM
> is at least
> `HBASE_SUPPORT` (7), which was added in 2024. So every OM that can receive
> this RPC already has the check.
> #
> ##
> ### Proposed change
> *isFileClosed.* When `proxy.getOmVersion() >= GET_FILE_STATUS_REJECTS_OBS`,
> call
> `proxy.getOzoneFileStatus(volume, bucket, key, false)` directly, the same way
> HDDS-15925 changed
> `getFileStatus`. Map `NOT_SUPPORTED_OPERATION` to `IllegalArgumentException`.
> Against an older OM keep the
> current `getBucket()` path.
> Keep the `isSnapshotPath()` check before the RPC. Today it runs after
> `getBucket()`, so a snapshot path in
> a missing bucket reports `FileNotFoundException`, and after the change it
> reports "file is in a
> snapshot". Either keep that order (cheap: the snapshot path check is local)
> or call the difference out
> in the PR.
> *recoverLease.* Drop the `getBucket()` and pass `ofsPath.getVolumeName()` /
> `getBucketName()`. Those are
> the names `bucket.getVolumeName()` / `getName()` return today, because the
> link handle carries the
> link's own names.
> The one behaviour change is the exception type for an OBJECT_STORE bucket.
> Today the client throws
> `IllegalArgumentException` before any RPC. Without the fetch, OM's plain
> `IOException` reaches the
> client as `OMException(INTERNAL_ERROR)`. To keep the exception type
> unchanged, follow the HDDS-16760
> pattern:
> * OM throws `OMException(NOT_SUPPORTED_OPERATION)` instead of `IOException`,
> with the same message.
> * Add a new `OzoneManagerVersion` constant, placed after
> `LIST_STATUS_REJECTS_OBS(101)`. The number is
> assigned at merge; coordinate it with sibling Jiras.
> * At or above that version the client skips `getBucket()` and maps
> `NOT_SUPPORTED_OPERATION` to
> `IllegalArgumentException`. Below it the client keeps `getBucket()`.
> LEGACY buckets behave as before: the client never rejected them, and OM
> rejects them under either code.
> *Compatibility*
> | |isFileClosed|recoverLease|
> |—|—|—|
> |New client, OM ≥ new constant|1 RPC, OBS → `IllegalArgumentException`|1 RPC,
> OBS → `IllegalArgumentException`|
> |New client, older OM|1 RPC for OM ≥ 14; `getBucket()` path
> below|`getBucket()` path, unchanged|
> |Old client, new OM|unchanged|unchanged, except LEGACY now gets
> `NOT_SUPPORTED_OPERATION` instead of `INTERNAL_ERROR`|
> There is no protobuf change and no new configuration.
> *Testing*
> * Adapter (Mockito partial mock, like
> `TestBasicRootedOzoneClientAdapterHeadOp`):
> * Both operations issue exactly one OM call and no `getBucket()`.
> *`NOT_SUPPORTED_OPERATION` → `IllegalArgumentException`.
> *`FILE_NOT_FOUND` / `KEY_NOT_FOUND` / `BUCKET_NOT_FOUND` keep today's
> `FileNotFoundException` mapping.
> * An older OM falls back to `getBucket()`, including an OM that reports
> `ZDU` for recoverLease.
> * OM: `TestOMRecoverLeaseRequest` (or the `createClientRequest` test)
> asserts `NOT_SUPPORTED_OPERATION`
> for OBJECT_STORE and LEGACY, and success for FSO.
> * `AbstractRootedOzoneFileSystemTest`: `isFileClosed` / `recoverLease` on an
> OBJECT_STORE bucket still fail
> with `IllegalArgumentException`. Assert the RPC-count property in the adapter
> test, not by OMMetrics
> deltas. OMMetrics is cluster-global and the trash emptier moves it
> (HDDS-16513).
> * Benchmark: the cost of these calls is too low and they are too rare to be
> worth a dedicated
> benchmark. The RPC count goes from 2 to 1 per call, and the adapter test
> asserts it exactly.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
