Skip to content

Conversation

@ivandika3
Copy link
Contributor

@ivandika3 ivandika3 commented Feb 20, 2024

What changes were proposed in this pull request?

Found an eTag incompatibility in S3 listParts while testing the multipart uploads an old S3G and a new OM.

Case 1: Old S3G and new OM

  1. issue is in KeyManagerImpl#listParts
    This will throw "java.util.NoSuchElementException: No value present" in case where the MPU part does not contain eTag field (before HDDS-9680)
OmPartInfo omPartInfo = new OmPartInfo(partKeyInfo.getPartNumber(),
    partName,
    partKeyInfo.getPartKeyInfo().getModificationTime(),
    partKeyInfo.getPartKeyInfo().getDataSize(),
    partKeyInfo.getPartKeyInfo().getMetadataList().stream()
        .filter(keyValue -> keyValue.getKey().equals(ETAG))
        .findFirst().get().getValue()); 
  1. ObjectEndpoint#listParts is currently only returning the MPU part eTag, which might not exist for old MPU parts. This can be resolved by falling back to using partName as eTag if the eTag is not specified.

  2. NPE when calling setETag with null (in OmPartInfo#getProto). This can be resolved by doing a simple nullity check.

Case 2: New S3G and old OM

  1. S3G does not return any eTag during MPU commit, causing InvalidPart during MPU complete. This can be resolved by falling back to use part name as eTag response, if OmMultipartCommitUploadPartInfo eTag field is empty / null.

Ongoing multipart uploads when OM is updated from the old OM to the new OM

There might be multipart uploads that are ongoing during the OM upgrades from the old OM (without md5 hash eTag) and new OM (with md5 hash eTag)

In the new OM, the eTag is calculated during the MPU complete using this method md5(<concatenated-part's-etag-or-partname>-<number of parts>), see S3MultipartUploadCompleteRequest#multipartUploadedKeyHash.

For example, for MPU done entirely on the new OM the eTag of the parts and the final key will be:

Part 1 ETAG (new): 6b3f1d2b18292bbb32585cf0ff31b36c
Part 2 ETAG (new): 6b3f1d2b18292bbb32585cf0ff31b36c
Part 3 ETAG (new): 6b3f1d2b18292bbb32585cf0ff31b36c

ETAG of MPU: md5(6b3f1d2b18292bbb32585cf0ff31b36c6b3f1d2b18292bbb32585cf0ff31b36c6b3f1d2b18292bbb32585cf0ff31b36c-3)

However, if some of the parts is uploaded before the new OM, it will fall back using the MPU part name instead. Hence the calculation will be

Part 1 ETAG (old): /s3v/etag-test-bucket/mpu-key-046d6d5b-eec1-41c2-bc97-6df8ebe12487-111968005487722496-1
Part 2 ETAG (old): /s3v/etag-test-bucket/mpu-key-046d6d5b-eec1-41c2-bc97-6df8ebe12487-111968005487722496-2
Part 3 ETAG (new): 6b3f1d2b18292bbb32585cf0ff31b36c

ETAG of MPU: md5(/s3v/etag-test-bucket/mpu-key-046d6d5b-eec1-41c2-bc97-6df8ebe12487-111968005487722496-1/s3v/etag-test-bucket/mpu-key-046d6d5b-eec1-41c2-bc97-6df8ebe12487-111968005487722496-26b3f1d2b18292bbb32585cf0ff31b36c-3).

There is a minor compatibility limitation, where another multipart upload with the same exact parts are uploaded again, the eTag will not be the same as the multipart upload uploaded during the OM upgrade. However, I think this should not have a major impact.

What is the link to the Apache JIRA

https://issues.apache.org/jira/browse/HDDS-10395

How was this patch tested?

Unit test and manual test between OM and S3G of different versions.

Clean CI run: https://github.com/ivandika3/ozone/actions/runs/7972393497

For reference, the issue replicated with the following MPU script which manually call the s3 API for MPU. It should also be able to be replicated with "aws s3 cp" for large files since it will use multipart uploads, and most probably will call the listParts API.

#!/bin/bash
BUCKET_NAME="etag-test-bucket"
KEY_NAME="mpu-key"
ENDPOINT_URL="S3_ENDPOINT"
AWS_ACCESS_KEY_ID="ACCESS_KEY_ID"
AWS_SECRET_ACCESS_KEY="SECRET_ACCESS_KEY"

# Create three files
dd if=/dev/urandom of=/tmp/part1 bs=1M count=10
dd if=/dev/urandom of=/tmp/part2 bs=1M count=10
dd if=/dev/urandom of=/tmp/part3 bs=1M count=10

# Define a function to conditionally add the endpoint url
function aws_cmd {
  if [[ -z "$ENDPOINT_URL" ]]; then
    AWS_ACCESS_KEY_ID=$AWS_ACCESS_KEY_ID AWS_SECRET_ACCESS_KEY=$AWS_SECRET_ACCESS_KEY aws s3api $@
  else
    AWS_ACCESS_KEY_ID=$AWS_ACCESS_KEY_ID AWS_SECRET_ACCESS_KEY=$AWS_SECRET_ACCESS_KEY aws s3api --endpoint-url $ENDPOINT_URL $@
  fi
}

# Start the multipart upload and get the upload ID
UPLOAD_ID=$(aws_cmd create-multipart-upload --bucket $BUCKET_NAME --key $KEY_NAME --query 'UploadId' --output text)
echo "upload id: $UPLOAD_ID"

# Upload the parts
ETAG1=$(aws_cmd upload-part --bucket $BUCKET_NAME --key $KEY_NAME --part-number 1 --body /tmp/part1 --upload-id $UPLOAD_ID --query 'ETag' --output text)
echo "ETAG1: $ETAG1"
ETAG2=$(aws_cmd upload-part --bucket $BUCKET_NAME --key $KEY_NAME --part-number 2 --body /tmp/part2 --upload-id $UPLOAD_ID --query 'ETag' --output text)
echo "ETAG2: $ETAG2"
ETAG3=$(aws_cmd upload-part --bucket $BUCKET_NAME --key $KEY_NAME --part-number 3 --body /tmp/part3 --upload-id $UPLOAD_ID --query 'ETag' --output text)
echo "ETAG3: $ETAG3"

# List the MPU parts (the issue was detected here)
aws_cmd list-parts --bucket $BUCKET_NAME --key $KEY_NAME --upload-id $UPLOAD_ID

aws_cmd complete-multipart-upload --multipart-upload "Parts=[{ETag=$ETAG1,PartNumber=1},{ETag=$ETAG2,PartNumber=2},{ETag=$ETAG3,PartNumber=3}]" --bucket $BUCKET_NAME --key $KEY_NAME --upload-id $UPLOAD_ID

@ivandika3 ivandika3 marked this pull request as ready for review February 20, 2024 13:56
@kerneltime
Copy link
Contributor

cc @SaketaChalamchala

@ivandika3
Copy link
Contributor Author

I encountered another compatibility issue during upload-part from the new S3G and old OM. Will add the patch after I tested it.

@kerneltime
Copy link
Contributor

I encountered another compatibility issue during upload-part from the new S3G and old OM. Will add the patch after I tested it.

Do you plan to add it to this PR?

@ivandika3
Copy link
Contributor Author

@kerneltime Yes, currently testing it.

@ivandika3 ivandika3 changed the title HDDS-10395. Fix compatibility issue with eTag during MPU listParts HDDS-10395. Fix eTag compatibility issues during MPU Feb 21, 2024
@ivandika3
Copy link
Contributor Author

ivandika3 commented Feb 21, 2024

@kerneltime I have tested the compatibility fixes manually and the result looks good. I also updated the PR title and the description with more information. PTAL. Thank you.

Copy link
Contributor

@myskov myskov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the patch @ivandika3.

@ivandika3
Copy link
Contributor Author

Note: I also noticed that the CopyObjectResponse's eTag field is still set wrongly (using random UUID). Filed HDDS-10403 for this.

@adoroszlai
Copy link
Contributor

@kerneltime would you like to take another look?

@adoroszlai adoroszlai merged commit 5f6306d into apache:master Feb 22, 2024
@adoroszlai
Copy link
Contributor

Thanks @ivandika3 for the patch, @kerneltime, @myskov for the review.

@ivandika3
Copy link
Contributor Author

Thank you for the reviews @kerneltime @myskov, and @adoroszlai for the merge.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants