You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by "ivandika3 (via GitHub)" <gi...@apache.org> on 2026/05/01 01:32:17 UTC
[PR] HDDS-12918. [ozone]
ivandika3 opened a new pull request, #10166:
URL: https://github.com/apache/ozone/pull/10166
## What changes were proposed in this pull request?
Similar to [HDDS-11699](https://issues.apache.org/jira/browse/HDDS-11699), we can move the byte range GET filter from S3G to the OM so that the network and serialization cost can be reduced for objects with a lot of blocks.
Applications like JuiceFS on S3 uses quite a bit of byte range fetch.
## What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-12918
## How was this patch tested?
UT, IT.
--
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-12918. OM support byte range GET [ozone]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #10166:
URL: https://github.com/apache/ozone/pull/10166#issuecomment-4930777944
Thank you for your contribution. This PR is being closed due to inactivity. Please contact a maintainer if you would like to reopen it.
--
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-12918. OM support byte range GET [ozone]
Posted by "ivandika3 (via GitHub)" <gi...@apache.org>.
ivandika3 commented on code in PR #10166:
URL: https://github.com/apache/ozone/pull/10166#discussion_r3198668243
##########
hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/endpoint/ObjectEndpoint.java:
##########
@@ -430,16 +446,19 @@ Response handleGetRequest(ObjectRequestContext context, String keyPath)
};
responseBuilder = Response.ok(output)
- .header(HttpHeaders.CONTENT_LENGTH, keyDetails.getDataSize());
+ .header(HttpHeaders.CONTENT_LENGTH, responseKeyDetails.getDataSize());
} else {
long startOffset = rangeHeader.getStartOffset();
long endOffset = rangeHeader.getEndOffset();
long copyLength = endOffset - startOffset + 1;
+ OzoneKeyDetails rangedKeyDetails = getClientProtocol()
+ .getS3KeyDetails(bucketName, keyPath, partNumber, startOffset,
+ endOffset);
Review Comment:
This is a correctness issue, we should not call getS3KeyDetails multiple times within a single S3 call since the latter getS3KeyDetails might not return the same key as the first (i.e. there is a key overwrite in between).
Addressing this.
--
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-12918. OM support byte range GET [ozone]
Posted by "Gargi-jais11 (via GitHub)" <gi...@apache.org>.
Gargi-jais11 commented on code in PR #10166:
URL: https://github.com/apache/ozone/pull/10166#discussion_r3179555611
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManagerImpl.java:
##########
@@ -647,9 +647,47 @@ private OmKeyInfo readKeyInfo(OmKeyArgs args, BucketLayout bucketLayout)
.sum());
}
}
+ if (args.hasByteRange()) {
+ filterKeyLocationsByByteRange(value, args.getByteRangeStart(),
+ args.getByteRangeEnd());
+ }
return value;
}
+ private void filterKeyLocationsByByteRange(OmKeyInfo keyInfo,
+ long byteRangeStart, long byteRangeEnd) {
+ OmKeyLocationInfoGroup latestLocationVersion =
+ keyInfo.getLatestVersionLocations();
+ if (latestLocationVersion == null) {
+ return;
+ }
+
+ List<OmKeyLocationInfo> currentLocations =
+ latestLocationVersion.getBlocksLatestVersionOnly();
+ List<OmKeyLocationInfo> filteredLocations = new ArrayList<>();
+ long blockStart = 0;
+ long firstBlockStart = 0;
+ for (OmKeyLocationInfo locationInfo : currentLocations) {
+ long blockEnd = blockStart + locationInfo.getLength() - 1;
+ if (blockStart > byteRangeEnd) {
+ break;
+ }
+ if (blockEnd >= byteRangeStart) {
+ if (filteredLocations.isEmpty()) {
+ firstBlockStart = blockStart;
+ }
+ filteredLocations.add(locationInfo);
+ }
+ blockStart += locationInfo.getLength();
+ }
+
+ keyInfo.updateLocationInfoList(filteredLocations,
+ latestLocationVersion.isMultipartKey(), true);
+ if (!filteredLocations.isEmpty()) {
+ keyInfo.setByteRangeStartOffset(byteRangeStart - firstBlockStart);
+ }
Review Comment:
I think we should also set `keyInfo.setDataSize (byteRangeEnd - byteRangeStart + 1)` as `OzoneKeyDetails` uses it`RpcClient.getOzoneKeyDetails(...)` does something like new `OzoneKeyDetails(..., keyInfo.getDataSize(), ...)`.
If you don’t update `dataSize` after filtering, `getDataSize()` still looks like the whole object / whole part, while `getContent() `only covers some blocks — confusing and wrong for anything that uses getDataSize() for buffers, metrics etc.
```suggestion
if (!filteredLocations.isEmpty()) {
keyInfo.setByteRangeStartOffset(byteRangeStart - firstBlockStart);
}
keyInfo.setDataSize (byteRangeEnd - byteRangeStart + 1)
```
##########
hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/rpc/RpcClient.java:
##########
@@ -1880,6 +1880,37 @@ public OzoneKeyDetails getS3KeyDetails(String bucketName, String keyName,
return getOzoneKeyDetails(keyInfo);
}
+ @Override
+ public OzoneKeyDetails getS3KeyDetails(String bucketName, String keyName,
+ int partNumber, long startOffset,
+ long endOffset) throws IOException {
Review Comment:
Nit: Please use 4 indent space
--
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-12918. OM support byte range GET [ozone]
Posted by "ivandika3 (via GitHub)" <gi...@apache.org>.
ivandika3 commented on code in PR #10166:
URL: https://github.com/apache/ozone/pull/10166#discussion_r3198668243
##########
hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/endpoint/ObjectEndpoint.java:
##########
@@ -430,16 +446,19 @@ Response handleGetRequest(ObjectRequestContext context, String keyPath)
};
responseBuilder = Response.ok(output)
- .header(HttpHeaders.CONTENT_LENGTH, keyDetails.getDataSize());
+ .header(HttpHeaders.CONTENT_LENGTH, responseKeyDetails.getDataSize());
} else {
long startOffset = rangeHeader.getStartOffset();
long endOffset = rangeHeader.getEndOffset();
long copyLength = endOffset - startOffset + 1;
+ OzoneKeyDetails rangedKeyDetails = getClientProtocol()
+ .getS3KeyDetails(bucketName, keyPath, partNumber, startOffset,
+ endOffset);
Review Comment:
This is a correctness issue, we should not call getS3KeyDetails multiple times within a single S3 call since the latter getS3KeyDetails might not return the same key as the first (i.e. there is a key overwrite in between).
The issue is that we need to move the range check to OM side.
Addressing this.
--
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-12918. OM support byte range GET [ozone]
Posted by "ivandika3 (via GitHub)" <gi...@apache.org>.
ivandika3 commented on code in PR #10166:
URL: https://github.com/apache/ozone/pull/10166#discussion_r3198668243
##########
hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/endpoint/ObjectEndpoint.java:
##########
@@ -430,16 +446,19 @@ Response handleGetRequest(ObjectRequestContext context, String keyPath)
};
responseBuilder = Response.ok(output)
- .header(HttpHeaders.CONTENT_LENGTH, keyDetails.getDataSize());
+ .header(HttpHeaders.CONTENT_LENGTH, responseKeyDetails.getDataSize());
} else {
long startOffset = rangeHeader.getStartOffset();
long endOffset = rangeHeader.getEndOffset();
long copyLength = endOffset - startOffset + 1;
+ OzoneKeyDetails rangedKeyDetails = getClientProtocol()
+ .getS3KeyDetails(bucketName, keyPath, partNumber, startOffset,
+ endOffset);
Review Comment:
This is a correctness issue, we should not call getS3KeyDetails multiple times within a single S3 call since the latter getS3KeyDetails might not return the same key as the first (i.e. there is a key overwrite in between).
The reason of the current multiple getS3KeyDetails is that the first getS3KeyDetails need to be used to get the key length for range header validation. Therefore, we need to push this range validation to OM.
Addressing this.
--
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-12918. OM support byte range GET [ozone]
Posted by "ivandika3 (via GitHub)" <gi...@apache.org>.
ivandika3 commented on code in PR #10166:
URL: https://github.com/apache/ozone/pull/10166#discussion_r3194798110
##########
hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/rpc/RpcClient.java:
##########
@@ -1880,6 +1880,37 @@ public OzoneKeyDetails getS3KeyDetails(String bucketName, String keyName,
return getOzoneKeyDetails(keyInfo);
}
+ @Override
+ public OzoneKeyDetails getS3KeyDetails(String bucketName, String keyName,
+ int partNumber, long startOffset,
+ long endOffset) throws IOException {
Review Comment:
Updated thanks.
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManagerImpl.java:
##########
@@ -647,9 +647,47 @@ private OmKeyInfo readKeyInfo(OmKeyArgs args, BucketLayout bucketLayout)
.sum());
}
}
+ if (args.hasByteRange()) {
+ filterKeyLocationsByByteRange(value, args.getByteRangeStart(),
+ args.getByteRangeEnd());
+ }
return value;
}
+ private void filterKeyLocationsByByteRange(OmKeyInfo keyInfo,
+ long byteRangeStart, long byteRangeEnd) {
+ OmKeyLocationInfoGroup latestLocationVersion =
+ keyInfo.getLatestVersionLocations();
+ if (latestLocationVersion == null) {
+ return;
+ }
+
+ List<OmKeyLocationInfo> currentLocations =
+ latestLocationVersion.getBlocksLatestVersionOnly();
+ List<OmKeyLocationInfo> filteredLocations = new ArrayList<>();
+ long blockStart = 0;
+ long firstBlockStart = 0;
+ for (OmKeyLocationInfo locationInfo : currentLocations) {
+ long blockEnd = blockStart + locationInfo.getLength() - 1;
+ if (blockStart > byteRangeEnd) {
+ break;
+ }
+ if (blockEnd >= byteRangeStart) {
+ if (filteredLocations.isEmpty()) {
+ firstBlockStart = blockStart;
+ }
+ filteredLocations.add(locationInfo);
+ }
+ blockStart += locationInfo.getLength();
+ }
+
+ keyInfo.updateLocationInfoList(filteredLocations,
+ latestLocationVersion.isMultipartKey(), true);
+ if (!filteredLocations.isEmpty()) {
+ keyInfo.setByteRangeStartOffset(byteRangeStart - firstBlockStart);
+ }
Review Comment:
Good catch, updated.
##########
hadoop-ozone/interface-client/src/main/proto/OmClientProtocol.proto:
##########
@@ -1101,6 +1101,11 @@ message KeyArgs {
// the given ETag for the operation to succeed. This is used for
// S3 conditional writes with the If-Match header.
optional string expectedETag = 24;
+
+ // Byte range requested by S3 GET. OM uses it to return only the
+ // key locations needed to satisfy the read.
+ optional uint64 byteRangeStart = 25;
+ optional uint64 byteRangeEnd = 26;
Review Comment:
Thanks, updated.
##########
hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/protocol/ClientProtocol.java:
##########
@@ -178,6 +179,49 @@ OzoneKeyDetails getS3KeyDetails(String bucketName, String keyName,
int partNumber)
throws IOException;
+ /**
+ * Get OzoneKey in S3 context with content positioned at the requested
+ * byte range start.
+ * @param bucketName Name of the Bucket
+ * @param keyName Key name
+ * @param partNumber Multipart-upload part number, or zero for the whole key
+ * @param startOffset inclusive byte range start offset
+ * @param endOffset inclusive byte range end offset
+ * @return {@link OzoneKey}
+ * @throws IOException
+ */
+ default OzoneKeyDetails getS3KeyDetails(String bucketName, String keyName,
Review Comment:
This exists as part of a client-side compatibility for other clients that implements `ClientProtocol`, but it's not `RpcClient`. If `getS3KeyDetails` API is called on older implementation (that does not have a concrete implementation of `getS3KeyDetails`) without, the API call will not work since it does not have any concrete implementation.
However, I think we can safely assume that only `RpcClient` implements `ClientProtocol` as Ozone is not a library per se (in contrast to Hadoop `FileSystem` interface which is extended depending on different backing store).
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/protocolPB/OzoneManagerRequestHandler.java:
##########
@@ -658,6 +663,11 @@ private GetKeyInfoResponse getKeyInfo(GetKeyInfoRequest request,
keyArgs.getForceUpdateContainerCacheFromSCM())
.setMultipartUploadPartNumber(keyArgs.getMultipartNumber())
.build();
+ if (keyArgs.hasByteRangeStart() && keyArgs.hasByteRangeEnd()) {
Review Comment:
Updated.
--
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-12918. OM support byte range GET [ozone]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] closed pull request #10166: HDDS-12918. OM support byte range GET
URL: https://github.com/apache/ozone/pull/10166
--
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-12918. OM support byte range GET [ozone]
Posted by "Gargi-jais11 (via GitHub)" <gi...@apache.org>.
Gargi-jais11 commented on code in PR #10166:
URL: https://github.com/apache/ozone/pull/10166#discussion_r3179589428
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManagerImpl.java:
##########
@@ -647,9 +647,47 @@ private OmKeyInfo readKeyInfo(OmKeyArgs args, BucketLayout bucketLayout)
.sum());
}
}
+ if (args.hasByteRange()) {
+ filterKeyLocationsByByteRange(value, args.getByteRangeStart(),
+ args.getByteRangeEnd());
+ }
return value;
}
+ private void filterKeyLocationsByByteRange(OmKeyInfo keyInfo,
+ long byteRangeStart, long byteRangeEnd) {
+ OmKeyLocationInfoGroup latestLocationVersion =
+ keyInfo.getLatestVersionLocations();
+ if (latestLocationVersion == null) {
+ return;
+ }
+
+ List<OmKeyLocationInfo> currentLocations =
+ latestLocationVersion.getBlocksLatestVersionOnly();
+ List<OmKeyLocationInfo> filteredLocations = new ArrayList<>();
+ long blockStart = 0;
+ long firstBlockStart = 0;
+ for (OmKeyLocationInfo locationInfo : currentLocations) {
+ long blockEnd = blockStart + locationInfo.getLength() - 1;
+ if (blockStart > byteRangeEnd) {
+ break;
+ }
+ if (blockEnd >= byteRangeStart) {
+ if (filteredLocations.isEmpty()) {
+ firstBlockStart = blockStart;
+ }
+ filteredLocations.add(locationInfo);
+ }
+ blockStart += locationInfo.getLength();
+ }
+
+ keyInfo.updateLocationInfoList(filteredLocations,
+ latestLocationVersion.isMultipartKey(), true);
+ if (!filteredLocations.isEmpty()) {
+ keyInfo.setByteRangeStartOffset(byteRangeStart - firstBlockStart);
+ }
Review Comment:
We can also add test assert for dataSize in `TestKeyManagerImpl` to check if it is set correctly or not.
--
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-12918. OM support byte range GET [ozone]
Posted by "peterxcli (via GitHub)" <gi...@apache.org>.
peterxcli commented on code in PR #10166:
URL: https://github.com/apache/ozone/pull/10166#discussion_r3180692841
##########
hadoop-ozone/interface-client/src/main/proto/OmClientProtocol.proto:
##########
@@ -1101,6 +1101,11 @@ message KeyArgs {
// the given ETag for the operation to succeed. This is used for
// S3 conditional writes with the If-Match header.
optional string expectedETag = 24;
+
+ // Byte range requested by S3 GET. OM uses it to return only the
+ // key locations needed to satisfy the read.
+ optional uint64 byteRangeStart = 25;
+ optional uint64 byteRangeEnd = 26;
Review Comment:
How about introducing a common msg, I remember few DN-SCM pb messages have range concept, maybe we could reuse theirs?
```proto
message ByteRange {
// Inclusive start and end offsets: [start, end].
optional uint64 start = 1;
optional uint64 end = 2;
}
message KeyArgs {
// ...
optional ByteRange byteRange = 25;
}
```
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/protocolPB/OzoneManagerRequestHandler.java:
##########
@@ -658,6 +663,11 @@ private GetKeyInfoResponse getKeyInfo(GetKeyInfoRequest request,
keyArgs.getForceUpdateContainerCacheFromSCM())
.setMultipartUploadPartNumber(keyArgs.getMultipartNumber())
.build();
+ if (keyArgs.hasByteRangeStart() && keyArgs.hasByteRangeEnd()) {
Review Comment:
could become
```suggestion
if (keyArgs.hasByteRange()) {
```
if we introduce new range message.
https://github.com/apache/ozone/pull/10166/changes#r3180692841
##########
hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/protocol/ClientProtocol.java:
##########
@@ -178,6 +179,49 @@ OzoneKeyDetails getS3KeyDetails(String bucketName, String keyName,
int partNumber)
throws IOException;
+ /**
+ * Get OzoneKey in S3 context with content positioned at the requested
+ * byte range start.
+ * @param bucketName Name of the Bucket
+ * @param keyName Key name
+ * @param partNumber Multipart-upload part number, or zero for the whole key
+ * @param startOffset inclusive byte range start offset
+ * @param endOffset inclusive byte range end offset
+ * @return {@link OzoneKey}
+ * @throws IOException
+ */
+ default OzoneKeyDetails getS3KeyDetails(String bucketName, String keyName,
Review Comment:
this should sit in ClientProtocolStub.
##########
hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/protocol/ClientProtocol.java:
##########
@@ -178,6 +179,49 @@ OzoneKeyDetails getS3KeyDetails(String bucketName, String keyName,
int partNumber)
throws IOException;
+ /**
+ * Get OzoneKey in S3 context with content positioned at the requested
+ * byte range start.
+ * @param bucketName Name of the Bucket
+ * @param keyName Key name
+ * @param partNumber Multipart-upload part number, or zero for the whole key
+ * @param startOffset inclusive byte range start offset
+ * @param endOffset inclusive byte range end offset
+ * @return {@link OzoneKey}
+ * @throws IOException
+ */
+ default OzoneKeyDetails getS3KeyDetails(String bucketName, String keyName,
Review Comment:
why there is a implementation?
--
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-12918. OM support byte range GET [ozone]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #10166:
URL: https://github.com/apache/ozone/pull/10166#issuecomment-4585255332
This PR has been marked as stale due to 21 days of inactivity. Please comment or remove the stale label to keep it open. Otherwise, it will be automatically closed in 7 days.
--
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