diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyCommitRequest.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyCommitRequest.java index 65b3394b5fcd..bb1a10095552 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyCommitRequest.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyCommitRequest.java @@ -642,24 +642,6 @@ protected void validateAtomicRewrite(OmKeyInfo existing, OmKeyInfo toCommit, Map } } } - - if (toCommit.getExpectedETag() != null) { - String expectedETag = toCommit.getExpectedETag(); - auditMap.put("expectedETag", expectedETag); - - if (existing == null) { - throw new OMException("Key not found for If-Match at commit", - OMException.ResultCodes.KEY_NOT_FOUND); - } - if (!existing.hasEtag()) { - throw new OMException("Key does not have an ETag at commit", - OMException.ResultCodes.ETAG_NOT_AVAILABLE); - } - if (!existing.isEtagEquals(expectedETag)) { - throw new OMException("ETag changed during write (concurrent modification)", - OMException.ResultCodes.ETAG_MISMATCH); - } - } } } diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyCreateRequest.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyCreateRequest.java index 4d37a1d7aa7f..b9cca39270c4 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyCreateRequest.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyCreateRequest.java @@ -256,6 +256,7 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, Execut OmKeyInfo dbKeyInfo = omMetadataManager.getKeyTable(getBucketLayout()) .getIfExist(dbKeyName); validateAtomicRewrite(dbKeyInfo, keyArgs); + keyArgs = validateAndRewriteIfMatchAsExpectedGeneration(keyArgs, dbKeyInfo); OmBucketInfo bucketInfo = getBucketInfo(omMetadataManager, volumeName, bucketName); @@ -497,20 +498,34 @@ protected void validateAtomicRewrite(OmKeyInfo dbKeyInfo, KeyArgs keyArgs) } } - if (keyArgs.hasExpectedETag()) { - String expectedETag = keyArgs.getExpectedETag(); - if (dbKeyInfo == null) { - throw new OMException("Key not found for If-Match", - OMException.ResultCodes.KEY_NOT_FOUND); - } - if (!dbKeyInfo.hasEtag()) { - throw new OMException("Key does not have an ETag", - OMException.ResultCodes.ETAG_NOT_AVAILABLE); - } - if (!dbKeyInfo.isEtagEquals(expectedETag)) { - throw new OMException("ETag mismatch", - OMException.ResultCodes.ETAG_MISMATCH); - } + } + + protected KeyArgs validateAndRewriteIfMatchAsExpectedGeneration( + KeyArgs keyArgs, OmKeyInfo dbKeyInfo) throws OMException { + if (!keyArgs.hasExpectedETag()) { + return keyArgs; + } + + String expectedETag = keyArgs.getExpectedETag(); + if (dbKeyInfo == null) { + throw new OMException("Key not found for If-Match", + OMException.ResultCodes.KEY_NOT_FOUND); } + if (!dbKeyInfo.hasEtag()) { + throw new OMException("Key does not have an ETag", + OMException.ResultCodes.ETAG_NOT_AVAILABLE); + } + if (!dbKeyInfo.isEtagEquals(expectedETag)) { + throw new OMException("ETag mismatch", + OMException.ResultCodes.ETAG_MISMATCH); + } + if (keyArgs.hasExpectedDataGeneration()) { + return keyArgs; + } + + return keyArgs.toBuilder() + .setExpectedDataGeneration(dbKeyInfo.getUpdateID()) + .clearExpectedETag() + .build(); } } diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyCreateRequestWithFSO.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyCreateRequestWithFSO.java index 980bb5871c3e..99fabb46de11 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyCreateRequestWithFSO.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyCreateRequestWithFSO.java @@ -122,6 +122,8 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, Execut omMetadataManager, dbFileKey, keyName); } validateAtomicRewrite(dbFileInfo, keyArgs); + keyArgs = validateAndRewriteIfMatchAsExpectedGeneration( + keyArgs, dbFileInfo); // Check if a file or directory exists with same key name. if (pathInfoFSO.getDirectoryResult() == DIRECTORY_EXISTS) { diff --git a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/key/TestOMKeyCommitRequest.java b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/key/TestOMKeyCommitRequest.java index 1f40b0f72a00..652a7aa0fcf1 100644 --- a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/key/TestOMKeyCommitRequest.java +++ b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/key/TestOMKeyCommitRequest.java @@ -350,143 +350,6 @@ public void testAtomicCreateIfNotExistsCommitKeyAlreadyExists() throws Exception assertEquals(KEY_ALREADY_EXISTS, omClientResponse.getOMResponse().getStatus()); } - @Test - public void testCommitWithExpectedETagSuccess() throws Exception { - Table openKeyTable = - omMetadataManager.getOpenKeyTable(getBucketLayout()); - Table closedKeyTable = - omMetadataManager.getKeyTable(getBucketLayout()); - - OMRequest modifiedOmRequest = - doPreExecute(createCommitKeyRequest()); - OMKeyCommitRequest omKeyCommitRequest = - getOmKeyCommitRequest(modifiedOmRequest); - KeyArgs keyArgs = - modifiedOmRequest.getCommitKeyRequest().getKeyArgs(); - - OMRequestTestUtils.addVolumeAndBucketToDB(volumeName, bucketName, - omMetadataManager, omKeyCommitRequest.getBucketLayout()); - - List allocatedLocationList = - keyArgs.getKeyLocationsList().stream() - .map(OmKeyLocationInfo::getFromProtobuf) - .collect(Collectors.toList()); - - String expectedETag = "matching-etag"; - OmKeyInfo.Builder omKeyInfoBuilder = OMRequestTestUtils.createOmKeyInfo( - volumeName, bucketName, keyName, replicationConfig, - new OmKeyLocationInfoGroup(version, new ArrayList<>())); - omKeyInfoBuilder.setExpectedETag(expectedETag); - - String openKey = addKeyToOpenKeyTable(allocatedLocationList, - omKeyInfoBuilder); - assertNotNull(openKeyTable.get(openKey)); - - // Add closed key with matching ETag - OmKeyInfo closedKeyInfo = OMRequestTestUtils.createOmKeyInfo( - volumeName, bucketName, keyName, replicationConfig, - new OmKeyLocationInfoGroup(version, new ArrayList<>())) - .addMetadata(OzoneConsts.ETAG, expectedETag).build(); - closedKeyTable.put(getOzonePathKey(), closedKeyInfo); - - OMClientResponse omClientResponse = - omKeyCommitRequest.validateAndUpdateCache(ozoneManager, 100L); - assertEquals(OK, omClientResponse.getOMResponse().getStatus()); - - OmKeyInfo committedKey = closedKeyTable.get(getOzonePathKey()); - assertNotNull(committedKey); - assertNull(committedKey.getExpectedETag()); - } - - @Test - public void testCommitWithExpectedETagMismatch() throws Exception { - Table openKeyTable = - omMetadataManager.getOpenKeyTable(getBucketLayout()); - Table closedKeyTable = - omMetadataManager.getKeyTable(getBucketLayout()); - - OMRequest modifiedOmRequest = - doPreExecute(createCommitKeyRequest()); - OMKeyCommitRequest omKeyCommitRequest = - getOmKeyCommitRequest(modifiedOmRequest); - KeyArgs keyArgs = - modifiedOmRequest.getCommitKeyRequest().getKeyArgs(); - - OMRequestTestUtils.addVolumeAndBucketToDB(volumeName, bucketName, - omMetadataManager, omKeyCommitRequest.getBucketLayout()); - - List allocatedLocationList = - keyArgs.getKeyLocationsList().stream() - .map(OmKeyLocationInfo::getFromProtobuf) - .collect(Collectors.toList()); - - OmKeyInfo.Builder omKeyInfoBuilder = OMRequestTestUtils.createOmKeyInfo( - volumeName, bucketName, keyName, replicationConfig, - new OmKeyLocationInfoGroup(version, new ArrayList<>())); - omKeyInfoBuilder.setExpectedETag("expected-etag"); - - String openKey = addKeyToOpenKeyTable(allocatedLocationList, - omKeyInfoBuilder); - assertNotNull(openKeyTable.get(openKey)); - - // Add closed key with different ETag - OmKeyInfo closedKeyInfo = OMRequestTestUtils.createOmKeyInfo( - volumeName, bucketName, keyName, replicationConfig, - new OmKeyLocationInfoGroup(version, new ArrayList<>())) - .addMetadata(OzoneConsts.ETAG, "different-etag").build(); - closedKeyTable.put(getOzonePathKey(), closedKeyInfo); - - OMClientResponse omClientResponse = - omKeyCommitRequest.validateAndUpdateCache(ozoneManager, 100L); - assertEquals( - OzoneManagerProtocolProtos.Status.ETAG_MISMATCH, - omClientResponse.getOMResponse().getStatus()); - } - - @Test - public void testCommitWithExpectedETagNoETagOnKey() throws Exception { - Table openKeyTable = - omMetadataManager.getOpenKeyTable(getBucketLayout()); - Table closedKeyTable = - omMetadataManager.getKeyTable(getBucketLayout()); - - OMRequest modifiedOmRequest = - doPreExecute(createCommitKeyRequest()); - OMKeyCommitRequest omKeyCommitRequest = - getOmKeyCommitRequest(modifiedOmRequest); - KeyArgs keyArgs = - modifiedOmRequest.getCommitKeyRequest().getKeyArgs(); - - OMRequestTestUtils.addVolumeAndBucketToDB(volumeName, bucketName, - omMetadataManager, omKeyCommitRequest.getBucketLayout()); - - List allocatedLocationList = - keyArgs.getKeyLocationsList().stream() - .map(OmKeyLocationInfo::getFromProtobuf) - .collect(Collectors.toList()); - - OmKeyInfo.Builder omKeyInfoBuilder = OMRequestTestUtils.createOmKeyInfo( - volumeName, bucketName, keyName, replicationConfig, - new OmKeyLocationInfoGroup(version, new ArrayList<>())); - omKeyInfoBuilder.setExpectedETag("expected-etag"); - - String openKey = addKeyToOpenKeyTable(allocatedLocationList, - omKeyInfoBuilder); - assertNotNull(openKeyTable.get(openKey)); - - // Add closed key WITHOUT ETag - OmKeyInfo closedKeyInfo = OMRequestTestUtils.createOmKeyInfo( - volumeName, bucketName, keyName, replicationConfig, - new OmKeyLocationInfoGroup(version, new ArrayList<>())).build(); - closedKeyTable.put(getOzonePathKey(), closedKeyInfo); - - OMClientResponse omClientResponse = - omKeyCommitRequest.validateAndUpdateCache(ozoneManager, 100L); - assertEquals( - OzoneManagerProtocolProtos.Status.ETAG_NOT_AVAILABLE, - omClientResponse.getOMResponse().getStatus()); - } - @Test public void testValidateAndUpdateCacheWithUncommittedBlocks() throws Exception { diff --git a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/key/TestOMKeyCreateRequest.java b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/key/TestOMKeyCreateRequest.java index f9364d87710d..800d86c503b6 100644 --- a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/key/TestOMKeyCreateRequest.java +++ b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/key/TestOMKeyCreateRequest.java @@ -340,11 +340,13 @@ public void testCreateWithExpectedETagSuccess( omKeyCreateRequest.validateAndUpdateCache(ozoneManager, 100L); assertEquals(OK, response.getOMResponse().getStatus()); - // Verify open key was created with expectedETag + // Verify open key was normalized onto the atomic rewrite generation. OmKeyInfo openKeyInfo = omMetadataManager.getOpenKeyTable(getBucketLayout()) .get(getOpenKey(id)); assertNotNull(openKeyInfo); - assertEquals(expectedETag, openKeyInfo.getExpectedETag()); + assertEquals(existingKeyInfo.getUpdateID(), + openKeyInfo.getExpectedDataGeneration()); + assertNull(openKeyInfo.getExpectedETag()); // Creation time should remain the same on rewrite assertEquals(existingKeyInfo.getCreationTime(), openKeyInfo.getCreationTime());