You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by "adoroszlai (via GitHub)" <gi...@apache.org> on 2024/10/28 11:21:11 UTC
[PR] HDDS-11607. Improve compatibility of listStatusLight [ozone]
adoroszlai opened a new pull request, #7369:
URL: https://github.com/apache/ozone/pull/7369
## What changes were proposed in this pull request?
Background:
- HDDS-7715 in 1.4.0 introduced `listStatusLight` request, used when listing keys in FSO buckets. Latest OM version was 4 at that time. OM version is not checked by client.
- HDDS-11414 in 2.0.0 introduced new OM version 8, and added client-side check to send `listStatusLight` only if OM version is >= 8, otherwise use older `listStatus` request.
- OM version = 8 cannot be backported to 1.4.x without backporting features that introduced versions 5-7. Therefore OM 1.4.x is stuck at version 4.
- OM version = 4 may or may not support `listStatusLight`, depending on whether it is 1.4.0 or some pre-release snapshot version.
- List keys fails with 1.4.0 client and 1.3.0 or earlier OM.
- HDDS-11414 fixed this compatibility bug by completely disabling `listStatusLight` in client, regardless of whether OM supports it or not.
This PR changes 2.0.0+ client to use `listStatusLight` with OM versions that support it (1.4.0 and newer). It can also be backported to 1.4.x, since it does not introduce a new OM version.
https://issues.apache.org/jira/browse/HDDS-11607
## How was this patch tested?
Tested with the commit preceding HDDS-7715. Implement a lightweight listStatus API:
- Built docker image with 68393452332a2d881c34eb2c21feee4b1a8e58e9.
- Set log level of client to DEBUG.
- Successfully ran `xcompat` with the custom docker image as old version.
- Verified FSO read compatibility test passes.
- Verified client log to confirm fallback to `listStatus`:
```
[main] DEBUG rpc.RpcClient: Failed listStatusLight, reverting to listStatus
```
Compatibility with other cluster versions are tested by CI:
https://github.com/adoroszlai/ozone/actions/runs/11550948011
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-11607. Improve compatibility of listStatusLight [ozone]
Posted by "xichen01 (via GitHub)" <gi...@apache.org>.
xichen01 commented on code in PR #7369:
URL: https://github.com/apache/ozone/pull/7369#discussion_r1826537399
##########
hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/rpc/RpcClient.java:
##########
@@ -2316,16 +2318,28 @@ public List<OzoneFileStatusLight> listStatusLight(String volumeName,
String bucketName, String keyName, boolean recursive, String startKey,
long numEntries, boolean allowPartialPrefixes) throws IOException {
OmKeyArgs keyArgs = prepareOmKeyArgs(volumeName, bucketName, keyName);
- if (omVersion.compareTo(OzoneManagerVersion.LIGHTWEIGHT_LIST_STATUS) >= 0) {
- return ozoneManagerClient.listStatusLight(keyArgs, recursive, startKey,
- numEntries, allowPartialPrefixes);
- } else {
- return ozoneManagerClient.listStatus(keyArgs, recursive, startKey,
- numEntries, allowPartialPrefixes)
- .stream()
- .map(OzoneFileStatusLight::fromOzoneFileStatus)
- .collect(Collectors.toList());
+ if (omVersion.compareTo(OzoneManagerVersion.MAYBE_LIGHTWEIGHT_LIST_STATUS) >= 0) {
+ try {
+ return ozoneManagerClient.listStatusLight(keyArgs, recursive, startKey,
Review Comment:
Maybe we can add caching of whether OM supports `listStatusLight`, the current implementation would at worst probably make every one `listStatusLight` at least twice RPC with the OM.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-11607. Improve compatibility of listStatusLight [ozone]
Posted by "sumitagrawl (via GitHub)" <gi...@apache.org>.
sumitagrawl commented on PR #7369:
URL: https://github.com/apache/ozone/pull/7369#issuecomment-2453725260
> Sure, I can abandon this change. We don't even need to backport it to 1.4.x, since `listStatusLight` is already disabled there ([6425679](https://github.com/apache/ozone/commit/6425679d71d414292153258841d3da79688b4ef2)). I think having V8 check in master and V4 check in 1.4.x is more confusing.
>
> The only "ugly" part of this change is having to keep V8 reserved, but that is local to the `OzoneManagerVersion` class. Other than that, this is the way [HDDS-11414](https://issues.apache.org/jira/browse/HDDS-11414) should have fixed the situation.
Yes, agree, do not need this in master. But can have in 1.4.x version.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-11607. Improve compatibility of listStatusLight [ozone]
Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.
adoroszlai commented on PR #7369:
URL: https://github.com/apache/ozone/pull/7369#issuecomment-2449160769
Sure, I can abandon this change. We don't even need to backport it to 1.4.x, since `listStatusLight` is already disabled there (6425679d71d414292153258841d3da79688b4ef2). I think having V8 check in master and V4 check in 1.4.x is more confusing.
The only "ugly" part of this change is having to keep V8 reserved, but that is local to the `OzoneManagerVersion` class. Other than that, this is the way HDDS-11414 should have fixed the situation.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-11607. Improve compatibility of listStatusLight [ozone]
Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.
adoroszlai closed pull request #7369: HDDS-11607. Improve compatibility of listStatusLight
URL: https://github.com/apache/ozone/pull/7369
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-11607. Improve compatibility of listStatusLight [ozone]
Posted by "xichen01 (via GitHub)" <gi...@apache.org>.
xichen01 commented on PR #7369:
URL: https://github.com/apache/ozone/pull/7369#issuecomment-2453327805
LGTM +1
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-11607. Improve compatibility of listStatusLight [ozone]
Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.
adoroszlai commented on code in PR #7369:
URL: https://github.com/apache/ozone/pull/7369#discussion_r1826582846
##########
hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/rpc/RpcClient.java:
##########
@@ -2316,16 +2318,28 @@ public List<OzoneFileStatusLight> listStatusLight(String volumeName,
String bucketName, String keyName, boolean recursive, String startKey,
long numEntries, boolean allowPartialPrefixes) throws IOException {
OmKeyArgs keyArgs = prepareOmKeyArgs(volumeName, bucketName, keyName);
- if (omVersion.compareTo(OzoneManagerVersion.LIGHTWEIGHT_LIST_STATUS) >= 0) {
- return ozoneManagerClient.listStatusLight(keyArgs, recursive, startKey,
- numEntries, allowPartialPrefixes);
- } else {
- return ozoneManagerClient.listStatus(keyArgs, recursive, startKey,
- numEntries, allowPartialPrefixes)
- .stream()
- .map(OzoneFileStatusLight::fromOzoneFileStatus)
- .collect(Collectors.toList());
+ if (omVersion.compareTo(OzoneManagerVersion.MAYBE_LIGHTWEIGHT_LIST_STATUS) >= 0) {
+ try {
+ return ozoneManagerClient.listStatusLight(keyArgs, recursive, startKey,
Review Comment:
Good idea. Please note that both version 4 (ad176eb0c10ced990d2063cb1b6d4004c77dbe55) and `listStatusLight` (6faf0023b057dc2cc51bdbf288a43e756fb42b4b) were both introduced in 1.4.0. Fallback is necessary only for those using Ozone based on any of the 118 commits in between these two.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-11607. Improve compatibility of listStatusLight [ozone]
Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.
adoroszlai commented on PR #7369:
URL: https://github.com/apache/ozone/pull/7369#issuecomment-2444091007
@errose28 @sumitagrawl @tanvipenumudy @xichen01 can you please review?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org