Skip to content

HDDS-10403. CopyObject should set ETag based on the key content #6251

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
adoroszlai merged 6 commits into from
Feb 23, 2024
Merged

HDDS-10403. CopyObject should set ETag based on the key content #6251

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
2a06bee
HDDS-10403. CopyObject should set eTag based on the key content
ivandika3 Feb 21, 2024
1280eb9
Fix objectcopy acceptance test
ivandika3 Feb 22, 2024
6106da1
Add eTag assertions for multipart copy test
ivandika3 Feb 22, 2024
717faea
Revert "Add eTag assertions for multipart copy test"
ivandika3 Feb 22, 2024
0e658f9
Remove redundant init
ivandika3 Feb 22, 2024
f9494e4
Remove another redundant init
ivandika3 Feb 22, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions hadoop-ozone/dist/src/main/smoketest/s3/objectcopy.robot
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,15 +37,26 @@ Create Dest Bucket
Copy Object Happy Scenario
Run Keyword if '${DESTBUCKET}' == 'generated1' Create Dest Bucket
Execute date > /tmp/copyfile
${file_checksum} = Execute md5sum /tmp/copyfile | awk '{print $1}'

${result} = Execute AWSS3ApiCli put-object --bucket ${BUCKET} --key ${PREFIX}/copyobject/key=value/f1 --body /tmp/copyfile
${eTag} = Execute and checkrc echo '${result}' | jq -r '.ETag' 0
Should Be Equal ${eTag} \"${file_checksum}\"

${result} = Execute AWSS3ApiCli list-objects --bucket ${BUCKET} --prefix ${PREFIX}/copyobject/key=value/
Should contain ${result} f1

${result} = Execute AWSS3ApiCli copy-object --bucket ${DESTBUCKET} --key ${PREFIX}/copyobject/key=value/f1 --copy-source ${BUCKET}/${PREFIX}/copyobject/key=value/f1
${eTag} = Execute and checkrc echo '${result}' | jq -r '.CopyObjectResult.ETag' 0
Should Be Equal ${eTag} \"${file_checksum}\"

${result} = Execute AWSS3ApiCli list-objects --bucket ${DESTBUCKET} --prefix ${PREFIX}/copyobject/key=value/
Should contain ${result} f1
#copying again will not throw error
${result} = Execute AWSS3ApiCli copy-object --bucket ${DESTBUCKET} --key ${PREFIX}/copyobject/key=value/f1 --copy-source ${BUCKET}/${PREFIX}/copyobject/key=value/f1
${eTag} = Execute and checkrc echo '${result}' | jq -r '.CopyObjectResult.ETag' 0
Should Be Equal ${eTag} \"${file_checksum}\"

${result} = Execute AWSS3ApiCli list-objects --bucket ${DESTBUCKET} --prefix ${PREFIX}/copyobject/key=value/
Should contain ${result} f1

Expand All @@ -56,8 +67,11 @@ Copy Object Where Bucket is not available
Should contain ${result} NoSuchBucket

Copy Object Where both source and dest are same with change to storageclass
${file_checksum} = Execute md5sum /tmp/copyfile | awk '{print $1}'
${result} = Execute AWSS3APICli copy-object --storage-class REDUCED_REDUNDANCY --bucket ${DESTBUCKET} --key ${PREFIX}/copyobject/key=value/f1 --copy-source ${DESTBUCKET}/${PREFIX}/copyobject/key=value/f1
Should contain ${result} ETag
${eTag} = Execute and checkrc echo '${result}' | jq -r '.CopyObjectResult.ETag' 0
Should Be Equal ${eTag} \"${file_checksum}\"

Copy Object Where Key not available
${result} = Execute AWSS3APICli and checkrc copy-object --bucket ${DESTBUCKET} --key ${PREFIX}/copyobject/key=value/f1 --copy-source ${BUCKET}/nonnonexistentkey 255
Expand Down
17 changes: 10 additions & 7 deletions hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/endpoint/ObjectEndpoint.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1109,13 +1109,14 @@ void copy(OzoneVolume volume, InputStream src, long srcKeyLen,
PerformanceStringBuilder perf, long startNanos)
throws IOException {
long copyLength;
src = new DigestInputStream(src, E_TAG_PROVIDER.get());
if (datastreamEnabled && !(replication != null &&
replication.getReplicationType() == EC) &&
srcKeyLen > datastreamMinLength) {
perf.appendStreamMode();
copyLength = ObjectEndpointStreaming
.copyKeyWithStream(volume.getBucket(destBucket), destKey, srcKeyLen,
chunkSize, replication, metadata, src, perf, startNanos);
chunkSize, replication, metadata, (DigestInputStream) src, perf, startNanos);
} else {
try (OzoneOutputStream dest = getClientProtocol()
.createKey(volume.getName(), destBucket, destKey, srcKeyLen,
Expand All @@ -1124,6 +1125,10 @@ void copy(OzoneVolume volume, InputStream src, long srcKeyLen,
getMetrics().updateCopyKeyMetadataStats(startNanos);
perf.appendMetaLatencyNanos(metadataLatencyNs);
copyLength = IOUtils.copyLarge(src, dest);
String eTag = DatatypeConverter.printHexBinary(
((DigestInputStream) src).getMessageDigest().digest())
.toLowerCase();
dest.getMetadata().put(ETAG, eTag);
}
}
getMetrics().incCopyObjectSuccessLength(copyLength);
Expand All @@ -1142,8 +1147,9 @@ private CopyObjectResponse copyObject(OzoneVolume volume,
String sourceBucket = result.getLeft();
String sourceKey = result.getRight();
try {
OzoneKeyDetails sourceKeyDetails = getClientProtocol().getKeyDetails(
volume.getName(), sourceBucket, sourceKey);
// Checking whether we trying to copying to it self.

if (sourceBucket.equals(destBucket) && sourceKey
.equals(destkey)) {
// When copying to same storage type when storage type is provided,
Expand All @@ -1162,15 +1168,12 @@ private CopyObjectResponse copyObject(OzoneVolume volume,
// still does not support this just returning dummy response
// for now
CopyObjectResponse copyObjectResponse = new CopyObjectResponse();
copyObjectResponse.setETag(OzoneUtils.getRequestID());
copyObjectResponse.setETag(wrapInQuotes(sourceKeyDetails.getMetadata().get(ETAG)));
copyObjectResponse.setLastModified(Instant.ofEpochMilli(
Time.now()));
return copyObjectResponse;
}
}

OzoneKeyDetails sourceKeyDetails = getClientProtocol().getKeyDetails(
volume.getName(), sourceBucket, sourceKey);
long sourceKeyLen = sourceKeyDetails.getDataSize();

try (OzoneInputStream src = getClientProtocol().getKey(volume.getName(),
Expand All @@ -1185,7 +1188,7 @@ private CopyObjectResponse copyObject(OzoneVolume volume,

getMetrics().updateCopyObjectSuccessStats(startNanos);
CopyObjectResponse copyObjectResponse = new CopyObjectResponse();
copyObjectResponse.setETag(OzoneUtils.getRequestID());
copyObjectResponse.setETag(wrapInQuotes(destKeyDetails.getMetadata().get(ETAG)));
copyObjectResponse.setLastModified(destKeyDetails.getModificationTime());
return copyObjectResponse;
} catch (OMException ex) {
Expand Down
9 changes: 6 additions & 3 deletions .../s3gateway/src/main/java/org/apache/hadoop/ozone/s3/endpoint/ObjectEndpointStreaming.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -122,15 +122,18 @@ public static long copyKeyWithStream(
int bufferSize,
ReplicationConfig replicationConfig,
Map<String, String> keyMetadata,
InputStream body, PerformanceStringBuilder perf, long startNanos)
DigestInputStream body, PerformanceStringBuilder perf, long startNanos)
throws IOException {
long writeLen = 0;
long writeLen;
try (OzoneDataStreamOutput streamOutput = bucket.createStreamKey(keyPath,
length, replicationConfig, keyMetadata)) {
long metadataLatencyNs =
METRICS.updateCopyKeyMetadataStats(startNanos);
perf.appendMetaLatencyNanos(metadataLatencyNs);
writeLen = writeToStreamOutput(streamOutput, body, bufferSize, length);
String eTag = DatatypeConverter.printHexBinary(body.getMessageDigest().digest())
.toLowerCase();
perf.appendMetaLatencyNanos(metadataLatencyNs);
((KeyMetadataAware)streamOutput).getMetadata().put(OzoneConsts.ETAG, eTag);
}
return writeLen;
}
Expand Down
2 changes: 1 addition & 1 deletion hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/client/OzoneBucketStub.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -209,7 +209,7 @@ public OzoneDataStreamOutput createStreamKey(String key, long size,
Map<String, String> keyMetadata)
throws IOException {
ByteBufferStreamOutput byteBufferStreamOutput =
new ByteBufferStreamOutput() {
new KeyMetadataAwareByteBufferStreamOutput(keyMetadata) {

private final ByteBuffer buffer = ByteBuffer.allocate((int) size);

Expand Down
27 changes: 27 additions & 0 deletions hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/endpoint/TestObjectPut.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,11 @@
import javax.ws.rs.core.HttpHeaders;
import javax.ws.rs.core.Response;
import org.apache.commons.io.IOUtils;
import org.apache.commons.lang3.StringUtils;
import org.apache.hadoop.hdds.client.ECReplicationConfig;
import org.apache.hadoop.hdds.client.ReplicationType;
import org.apache.hadoop.hdds.conf.OzoneConfiguration;
import org.apache.hadoop.ozone.OzoneConsts;
import org.apache.hadoop.ozone.client.ObjectStore;
import org.apache.hadoop.ozone.client.OzoneBucket;
import org.apache.hadoop.ozone.client.OzoneClient;
Expand All @@ -52,7 +54,9 @@
import static org.apache.hadoop.ozone.s3.util.S3Utils.urlEncode;
import static org.assertj.core.api.Assertions.assertThat;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.mockito.Mockito.any;
import static org.mockito.Mockito.doThrow;
import static org.mockito.Mockito.eq;
Expand Down Expand Up @@ -108,9 +112,12 @@ public void testPutObject() throws IOException, OS3Exception {
.readKey(keyName);
String keyContent =
IOUtils.toString(ozoneInputStream, UTF_8);
OzoneKeyDetails keyDetails = clientStub.getObjectStore().getS3Bucket(bucketName).getKey(keyName);

assertEquals(200, response.getStatus());
assertEquals(CONTENT, keyContent);
assertNotNull(keyDetails.getMetadata());
assertTrue(StringUtils.isNotEmpty(keyDetails.getMetadata().get(OzoneConsts.ETAG)));
}

@Test
Expand All @@ -136,9 +143,12 @@ public void testPutObjectWithECReplicationConfig()
.readKey(keyName);
String keyContent =
IOUtils.toString(ozoneInputStream, UTF_8);
OzoneKeyDetails keyDetails = clientStub.getObjectStore().getS3Bucket(bucketName).getKey(keyName);

assertEquals(200, response.getStatus());
assertEquals(CONTENT, keyContent);
assertNotNull(keyDetails.getMetadata());
assertTrue(StringUtils.isNotEmpty(keyDetails.getMetadata().get(OzoneConsts.ETAG)));
}

@Test
Expand Down Expand Up @@ -208,9 +218,12 @@ public void testPutObjectWithSignedChunks() throws IOException, OS3Exception {
clientStub.getObjectStore().getS3Bucket(bucketName)
.readKey(keyName);
String keyContent = IOUtils.toString(ozoneInputStream, UTF_8);
OzoneKeyDetails keyDetails = clientStub.getObjectStore().getS3Bucket(bucketName).getKey(keyName);

assertEquals(200, response.getStatus());
assertEquals("1234567890abcde", keyContent);
assertNotNull(keyDetails.getMetadata());
assertTrue(StringUtils.isNotEmpty(keyDetails.getMetadata().get(OzoneConsts.ETAG)));
}

@Test
Expand All @@ -230,10 +243,14 @@ public void testCopyObject() throws IOException, OS3Exception {
.readKey(keyName);

String keyContent = IOUtils.toString(ozoneInputStream, UTF_8);
OzoneKeyDetails keyDetails = clientStub.getObjectStore().getS3Bucket(bucketName).getKey(keyName);

assertEquals(200, response.getStatus());
assertEquals(CONTENT, keyContent);
assertNotNull(keyDetails.getMetadata());
assertTrue(StringUtils.isNotEmpty(keyDetails.getMetadata().get(OzoneConsts.ETAG)));

String sourceETag = keyDetails.getMetadata().get(OzoneConsts.ETAG);

// Add copy header, and then call put
when(headers.getHeaderString(COPY_SOURCE_HEADER)).thenReturn(
Expand All @@ -247,9 +264,19 @@ public void testCopyObject() throws IOException, OS3Exception {
.readKey(destkey);

keyContent = IOUtils.toString(ozoneInputStream, UTF_8);
OzoneKeyDetails sourceKeyDetails = clientStub.getObjectStore()
.getS3Bucket(bucketName).getKey(keyName);
OzoneKeyDetails destKeyDetails = clientStub.getObjectStore()
.getS3Bucket(destBucket).getKey(destkey);

assertEquals(200, response.getStatus());
assertEquals(CONTENT, keyContent);
assertNotNull(keyDetails.getMetadata());
assertTrue(StringUtils.isNotEmpty(keyDetails.getMetadata().get(OzoneConsts.ETAG)));
// Source key eTag should remain unchanged and the dest key should have
// the same Etag since the key content is the same
assertEquals(sourceETag, sourceKeyDetails.getMetadata().get(OzoneConsts.ETAG));
assertEquals(sourceETag, destKeyDetails.getMetadata().get(OzoneConsts.ETAG));

// source and dest same
OS3Exception e = assertThrows(OS3Exception.class, () -> objectEndpoint.put(
Expand Down