ArafatKhan2198 commented on code in PR #11020:
URL: https://github.com/apache/ozone/pull/11020#discussion_r3878720533
##########
hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/endpoint/ObjectAttributesHandler.java:
##########
@@ -170,4 +173,89 @@ private GetObjectAttributesResponse buildResponse(OzoneKey
key, Set<String> requ
return resp;
}
+
+ /**
+ * Builds the {@link GetObjectAttributesResponse.ObjectParts} element for a
completed
+ * multipart object, including per-part sizes and optional pagination.
+ *
+ * <p>When {@code x-amz-max-parts} is omitted, the page size defaults to
1000, matching ListParts.
+ * Each part size is fetched via a part-aware {@code headS3Object} call (one
OM RPC per part
+ * in the current page).
+ */
+ private GetObjectAttributesResponse.ObjectParts buildObjectParts(String
bucketName,
+ String keyPath, int totalPartsCount, String resource) throws
IOException, OS3Exception {
+ Integer maxPartsHeader = parseMaxPartsHeader(resource);
+ Integer partNumberMarker = parsePartNumberMarkerHeader(resource);
+ int marker = partNumberMarker != null ? partNumberMarker : 0;
+ int maxParts = maxPartsHeader != null
+ ? maxPartsHeader : GET_OBJECT_ATTRIBUTES_MAX_PARTS_LIMIT;
+
+ GetObjectAttributesResponse.ObjectParts parts = new
GetObjectAttributesResponse.ObjectParts();
+ parts.setPartsCount(totalPartsCount);
+ parts.setMaxParts(maxParts);
+ if (partNumberMarker != null) {
+ parts.setPartNumberMarker(partNumberMarker);
+ }
+
+ int lastPartReturned = marker;
+ for (int partNumber = marker + 1;
+ partNumber <= totalPartsCount && parts.getParts().size() < maxParts;
+ partNumber++) {
+ OzoneKey partKey;
+ try {
+ partKey = getClientProtocol().headS3Object(bucketName, keyPath,
partNumber);
Review Comment:
This implementation sends one OM request for every part. With the default
`max-parts` value of 1000, one `GetObjectAttributes` request can result in
around 1001 sequential OM calls, including the initial object lookup.
This may cause high latency in S3 Gateway and heavy load on OM. It can also
return inconsistent information if the object is overwritten while these calls
are running.
Can we provide one bulk or paginated OM operation that returns the part
number and size for all parts in the requested page?
##########
hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/endpoint/ObjectAttributesHandler.java:
##########
@@ -170,4 +173,89 @@ private GetObjectAttributesResponse buildResponse(OzoneKey
key, Set<String> requ
return resp;
}
+
+ /**
+ * Builds the {@link GetObjectAttributesResponse.ObjectParts} element for a
completed
+ * multipart object, including per-part sizes and optional pagination.
+ *
+ * <p>When {@code x-amz-max-parts} is omitted, the page size defaults to
1000, matching ListParts.
+ * Each part size is fetched via a part-aware {@code headS3Object} call (one
OM RPC per part
+ * in the current page).
+ */
+ private GetObjectAttributesResponse.ObjectParts buildObjectParts(String
bucketName,
+ String keyPath, int totalPartsCount, String resource) throws
IOException, OS3Exception {
+ Integer maxPartsHeader = parseMaxPartsHeader(resource);
+ Integer partNumberMarker = parsePartNumberMarkerHeader(resource);
+ int marker = partNumberMarker != null ? partNumberMarker : 0;
+ int maxParts = maxPartsHeader != null
+ ? maxPartsHeader : GET_OBJECT_ATTRIBUTES_MAX_PARTS_LIMIT;
+
+ GetObjectAttributesResponse.ObjectParts parts = new
GetObjectAttributesResponse.ObjectParts();
+ parts.setPartsCount(totalPartsCount);
+ parts.setMaxParts(maxParts);
+ if (partNumberMarker != null) {
+ parts.setPartNumberMarker(partNumberMarker);
+ }
+
+ int lastPartReturned = marker;
+ for (int partNumber = marker + 1;
+ partNumber <= totalPartsCount && parts.getParts().size() < maxParts;
+ partNumber++) {
+ OzoneKey partKey;
+ try {
+ partKey = getClientProtocol().headS3Object(bucketName, keyPath,
partNumber);
+ } catch (OMException ex) {
+ if (ex.getResult() == ResultCodes.KEY_NOT_FOUND) {
+ throw newError(NO_SUCH_KEY, keyPath, ex);
+ } else if (isAccessDenied(ex)) {
+ throw newError(ACCESS_DENIED, bucketName + "/" + keyPath, ex);
+ }
+ throw newError(resource, ex);
+ }
+ parts.addPart(new GetObjectAttributesResponse.Part(partNumber,
partKey.getDataSize()));
+ lastPartReturned = partNumber;
+ }
+
+ boolean truncated = lastPartReturned < totalPartsCount;
+ parts.setTruncated(truncated);
+ if (truncated) {
+ parts.setNextPartNumberMarker(lastPartReturned);
+ }
+ return parts;
+ }
+
+ private Integer parseMaxPartsHeader(String resource) throws OS3Exception {
+ String headerValue = getHeaders().getHeaderString(MAX_PARTS_HEADER);
+ if (StringUtils.isBlank(headerValue)) {
+ return null;
+ }
+ try {
+ int maxParts = Integer.parseInt(headerValue.trim());
+ if (maxParts <= 0 || maxParts > GET_OBJECT_ATTRIBUTES_MAX_PARTS_LIMIT) {
+ throw newError(INVALID_ARGUMENT, resource,
+ new IllegalArgumentException("max-parts must be between 1 and "
+ + GET_OBJECT_ATTRIBUTES_MAX_PARTS_LIMIT));
+ }
+ return maxParts;
+ } catch (NumberFormatException ex) {
+ throw newError(INVALID_ARGUMENT, resource, ex);
+ }
+ }
+
+ private Integer parsePartNumberMarkerHeader(String resource) throws
OS3Exception {
+ String headerValue =
getHeaders().getHeaderString(PART_NUMBER_MARKER_HEADER);
+ if (StringUtils.isBlank(headerValue)) {
+ return null;
+ }
+ try {
+ int marker = Integer.parseInt(headerValue.trim());
+ if (marker < 0) {
Review Comment:
The marker currently has no upper limit. If the client sends
`Integer.MAX_VALUE`, the expression `marker + 1` will overflow and become a
negative number.
This may cause negative part numbers or incorrect part sizes in the response.
Can we validate that `part-number-marker` is between 0 and 10,000 before
using it? Please also add tests for 10,000, 10,001, and `Integer.MAX_VALUE`.
##########
hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/endpoint/ObjectAttributesHandler.java:
##########
@@ -170,4 +173,89 @@ private GetObjectAttributesResponse buildResponse(OzoneKey
key, Set<String> requ
return resp;
}
+
+ /**
+ * Builds the {@link GetObjectAttributesResponse.ObjectParts} element for a
completed
+ * multipart object, including per-part sizes and optional pagination.
+ *
+ * <p>When {@code x-amz-max-parts} is omitted, the page size defaults to
1000, matching ListParts.
+ * Each part size is fetched via a part-aware {@code headS3Object} call (one
OM RPC per part
+ * in the current page).
+ */
+ private GetObjectAttributesResponse.ObjectParts buildObjectParts(String
bucketName,
+ String keyPath, int totalPartsCount, String resource) throws
IOException, OS3Exception {
+ Integer maxPartsHeader = parseMaxPartsHeader(resource);
+ Integer partNumberMarker = parsePartNumberMarkerHeader(resource);
+ int marker = partNumberMarker != null ? partNumberMarker : 0;
+ int maxParts = maxPartsHeader != null
+ ? maxPartsHeader : GET_OBJECT_ATTRIBUTES_MAX_PARTS_LIMIT;
+
+ GetObjectAttributesResponse.ObjectParts parts = new
GetObjectAttributesResponse.ObjectParts();
+ parts.setPartsCount(totalPartsCount);
+ parts.setMaxParts(maxParts);
+ if (partNumberMarker != null) {
+ parts.setPartNumberMarker(partNumberMarker);
+ }
+
+ int lastPartReturned = marker;
+ for (int partNumber = marker + 1;
Review Comment:
I think there is a correctness issue here. `totalPartsCount` tells us how
many parts exist, but it does not tell us the highest part number.
Ozone supports completed multipart objects with non-contiguous part numbers,
for example parts 1 and 3. In that case, the total part count is 2, but this
loop tries to fetch parts 1 and 2. Part 2 will fail with `InvalidPart`, and
part 3 will never be returned.
Can we fetch the actual completed part numbers from OM and paginate over
those part numbers instead? Please also add a test using non-contiguous parts
such as 1 and 3.
##########
hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/endpoint/ObjectAttributesHandler.java:
##########
@@ -170,4 +173,89 @@ private GetObjectAttributesResponse buildResponse(OzoneKey
key, Set<String> requ
return resp;
}
+
+ /**
+ * Builds the {@link GetObjectAttributesResponse.ObjectParts} element for a
completed
+ * multipart object, including per-part sizes and optional pagination.
+ *
+ * <p>When {@code x-amz-max-parts} is omitted, the page size defaults to
1000, matching ListParts.
+ * Each part size is fetched via a part-aware {@code headS3Object} call (one
OM RPC per part
+ * in the current page).
+ */
+ private GetObjectAttributesResponse.ObjectParts buildObjectParts(String
bucketName,
+ String keyPath, int totalPartsCount, String resource) throws
IOException, OS3Exception {
+ Integer maxPartsHeader = parseMaxPartsHeader(resource);
+ Integer partNumberMarker = parsePartNumberMarkerHeader(resource);
+ int marker = partNumberMarker != null ? partNumberMarker : 0;
+ int maxParts = maxPartsHeader != null
+ ? maxPartsHeader : GET_OBJECT_ATTRIBUTES_MAX_PARTS_LIMIT;
+
+ GetObjectAttributesResponse.ObjectParts parts = new
GetObjectAttributesResponse.ObjectParts();
+ parts.setPartsCount(totalPartsCount);
+ parts.setMaxParts(maxParts);
+ if (partNumberMarker != null) {
+ parts.setPartNumberMarker(partNumberMarker);
+ }
+
+ int lastPartReturned = marker;
+ for (int partNumber = marker + 1;
+ partNumber <= totalPartsCount && parts.getParts().size() < maxParts;
+ partNumber++) {
+ OzoneKey partKey;
+ try {
+ partKey = getClientProtocol().headS3Object(bucketName, keyPath,
partNumber);
+ } catch (OMException ex) {
+ if (ex.getResult() == ResultCodes.KEY_NOT_FOUND) {
+ throw newError(NO_SUCH_KEY, keyPath, ex);
+ } else if (isAccessDenied(ex)) {
+ throw newError(ACCESS_DENIED, bucketName + "/" + keyPath, ex);
+ }
+ throw newError(resource, ex);
+ }
+ parts.addPart(new GetObjectAttributesResponse.Part(partNumber,
partKey.getDataSize()));
Review Comment:
Could you please confirm the expected AWS compatibility here?
AWS documentation says that for general-purpose buckets, the `Part` elements
are not returned when the object does not have an additional checksum. Ozone
currently does not store these additional checksums, but this implementation
always returns the part entries.
If this is an intentional Ozone difference, we should document it.
Otherwise, the part entries may need to be returned only when the required
checksum information is available.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
