diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMOpenKeysDeleteRequest.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMOpenKeysDeleteRequest.java index 5e9c256d986f..197da11041f2 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMOpenKeysDeleteRequest.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMOpenKeysDeleteRequest.java @@ -97,7 +97,7 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, } omClientResponse = new OMOpenKeysDeleteResponse(omResponse.build(), - deletedOpenKeys, ozoneManager.isRatisEnabled()); + deletedOpenKeys, ozoneManager.isRatisEnabled(), getBucketLayout()); result = Result.SUCCESS; } catch (IOException ex) { @@ -151,8 +151,7 @@ private void updateOpenKeyTableCache(OzoneManager ozoneManager, volumeName, bucketName); for (OpenKey key: keysPerBucket.getKeysList()) { - String fullKeyName = omMetadataManager.getOpenKey(volumeName, - bucketName, key.getName(), key.getClientID()); + String fullKeyName = key.getName(); // If an open key is no longer present in the table, it was committed // and should not be deleted. diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/response/key/OMOpenKeysDeleteResponse.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/response/key/OMOpenKeysDeleteResponse.java index 4b52ea04725e..d77a91bad319 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/response/key/OMOpenKeysDeleteResponse.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/response/key/OMOpenKeysDeleteResponse.java @@ -23,7 +23,7 @@ import org.apache.hadoop.ozone.om.helpers.BucketLayout; import org.apache.hadoop.ozone.om.helpers.OmKeyInfo; import org.apache.hadoop.ozone.om.response.CleanupTableInfo; -import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos; +import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.OMResponse; import javax.annotation.Nonnull; import java.io.IOException; @@ -43,10 +43,12 @@ public class OMOpenKeysDeleteResponse extends AbstractOMKeyDeleteResponse { private Map keysToDelete; public OMOpenKeysDeleteResponse( - @Nonnull OzoneManagerProtocolProtos.OMResponse omResponse, - @Nonnull Map keysToDelete, boolean isRatisEnabled) { + @Nonnull OMResponse omResponse, + @Nonnull Map keysToDelete, + boolean isRatisEnabled, + @Nonnull BucketLayout bucketLayout) { - super(omResponse, isRatisEnabled); + super(omResponse, isRatisEnabled, bucketLayout); this.keysToDelete = keysToDelete; } @@ -55,8 +57,8 @@ public OMOpenKeysDeleteResponse( * For a successful request, the other constructor should be used. */ public OMOpenKeysDeleteResponse( - @Nonnull OzoneManagerProtocolProtos.OMResponse omResponse, @Nonnull - BucketLayout bucketLayout) { + @Nonnull OMResponse omResponse, + @Nonnull BucketLayout bucketLayout) { super(omResponse, bucketLayout); } diff --git a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/key/TestOMOpenKeysDeleteRequest.java b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/key/TestOMOpenKeysDeleteRequest.java index 8e1fa07f1fc0..74abefc2355a 100644 --- a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/key/TestOMOpenKeysDeleteRequest.java +++ b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/key/TestOMOpenKeysDeleteRequest.java @@ -20,11 +20,19 @@ import java.util.ArrayList; import java.util.Arrays; +import java.util.Collection; import java.util.List; import java.util.UUID; +import java.util.stream.Collectors; +import org.apache.commons.lang3.tuple.Pair; +import org.apache.hadoop.hdds.client.ReplicationConfig; +import org.apache.hadoop.hdds.client.ReplicationFactor; +import org.apache.hadoop.hdds.client.ReplicationType; import org.apache.hadoop.ozone.om.OMMetrics; +import org.apache.hadoop.ozone.om.helpers.BucketLayout; import org.apache.hadoop.ozone.om.helpers.OmKeyInfo; +import org.apache.hadoop.ozone.om.helpers.OzoneFSUtils; import org.apache.hadoop.ozone.om.request.OMRequestTestUtils; import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos; import org.junit.Assert; @@ -41,11 +49,35 @@ .OpenKeyBucket; import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos .OMRequest; +import org.junit.runner.RunWith; +import org.junit.runners.Parameterized; +import org.junit.runners.Parameterized.Parameters; /** * Tests OMOpenKeysDeleteRequest. */ +@RunWith(Parameterized.class) public class TestOMOpenKeysDeleteRequest extends TestOMKeyRequest { + + private final BucketLayout bucketLayout; + + public TestOMOpenKeysDeleteRequest(BucketLayout bucketLayout) { + this.bucketLayout = bucketLayout; + } + + @Override + public BucketLayout getBucketLayout() { + return bucketLayout; + } + + @Parameters + public static Collection bucketLayouts() { + return Arrays.asList( + BucketLayout.DEFAULT, + BucketLayout.FILE_SYSTEM_OPTIMIZED + ); + } + /** * Tests removing keys from the open key table cache that never existed there. * The operation should complete without errors. @@ -58,7 +90,8 @@ public class TestOMOpenKeysDeleteRequest extends TestOMKeyRequest { */ @Test public void testDeleteOpenKeysNotInTable() throws Exception { - OpenKeyBucket openKeys = makeOpenKeys(volumeName, bucketName, 5); + List> openKeys = + makeOpenKeys(volumeName, bucketName, 5); deleteOpenKeysFromCache(openKeys); assertNotInOpenKeyTable(openKeys); } @@ -78,14 +111,20 @@ public void testDeleteSubsetOfOpenKeys() throws Exception { final String bucket1 = UUID.randomUUID().toString(); final String bucket2 = UUID.randomUUID().toString(); - OpenKeyBucket v1b1KeysToDelete = makeOpenKeys(volume1, bucket1, 3); - OpenKeyBucket v1b1KeysToKeep = makeOpenKeys(volume1, bucket1, 3); + List> v1b1KeysToDelete = + makeOpenKeys(volume1, bucket1, 3); + List> v1b1KeysToKeep = + makeOpenKeys(volume1, bucket1, 3); - OpenKeyBucket v1b2KeysToDelete = makeOpenKeys(volume1, bucket2, 3); - OpenKeyBucket v1b2KeysToKeep = makeOpenKeys(volume1, bucket2, 2); + List> v1b2KeysToDelete = + makeOpenKeys(volume1, bucket2, 3); + List> v1b2KeysToKeep = + makeOpenKeys(volume1, bucket2, 2); - OpenKeyBucket v2b2KeysToDelete = makeOpenKeys(volume2, bucket2, 2); - OpenKeyBucket v2b2KeysToKeep = makeOpenKeys(volume2, bucket2, 3); + List> v2b2KeysToDelete = + makeOpenKeys(volume2, bucket2, 2); + List> v2b2KeysToKeep = + makeOpenKeys(volume2, bucket2, 3); addToOpenKeyTableDB( v1b1KeysToKeep, @@ -124,11 +163,12 @@ public void testDeleteSubsetOfOpenKeys() throws Exception { public void testDeleteSameKeyNameDifferentClient() throws Exception { final String volume = UUID.randomUUID().toString(); final String bucket = UUID.randomUUID().toString(); + final String key = UUID.randomUUID().toString(); - OpenKeyBucket keysToKeep = - makeOpenKeys(volume, bucket, 3, true); - OpenKeyBucket keysToDelete = - makeOpenKeys(volume, bucket, 3, true); + List> keysToKeep = + makeOpenKeys(volume, bucket, key, 3); + List> keysToDelete = + makeOpenKeys(volume, bucket, key, 3); addToOpenKeyTableDB(keysToKeep, keysToDelete); deleteOpenKeysFromCache(keysToDelete); @@ -146,6 +186,9 @@ public void testDeleteSameKeyNameDifferentClient() throws Exception { */ @Test public void testMetrics() throws Exception { + final String volume = UUID.randomUUID().toString(); + final String bucket = UUID.randomUUID().toString(); + final String key = UUID.randomUUID().toString(); final int numExistentKeys = 3; final int numNonExistentKeys = 5; @@ -155,10 +198,10 @@ public void testMetrics() throws Exception { Assert.assertEquals(metrics.getNumOpenKeysSubmittedForDeletion(), 0); Assert.assertEquals(metrics.getNumOpenKeysDeleted(), 0); - OpenKeyBucket existentKeys = - makeOpenKeys(volumeName, bucketName, numExistentKeys, true); - OpenKeyBucket nonExistentKeys = - makeOpenKeys(volumeName, bucketName, numNonExistentKeys, true); + List> existentKeys = + makeOpenKeys(volume, bucket, key, numExistentKeys); + List> nonExistentKeys = + makeOpenKeys(volume, bucket, key, numNonExistentKeys); addToOpenKeyTableDB(existentKeys); deleteOpenKeysFromCache(existentKeys, nonExistentKeys); @@ -180,7 +223,15 @@ public void testMetrics() throws Exception { * Asserts that the call's response status is {@link Status#OK}. * @throws Exception */ - private void deleteOpenKeysFromCache(OpenKeyBucket... openKeys) + private void deleteOpenKeysFromCache(List>... allKeys) + throws Exception { + + deleteOpenKeysFromCache(Arrays.stream(allKeys) + .flatMap(List::stream) + .collect(Collectors.toList())); + } + + private void deleteOpenKeysFromCache(List> openKeys) throws Exception { OMRequest omRequest = @@ -202,130 +253,123 @@ private void deleteOpenKeysFromCache(OpenKeyBucket... openKeys) * are present after the addition. * @throws Exception */ - private void addToOpenKeyTableDB(OpenKeyBucket... openKeys) + private void addToOpenKeyTableDB(List>... allKeys) throws Exception { - addToOpenKeyTableDB(0, openKeys); + addToOpenKeyTableDB(0, Arrays.stream(allKeys) + .flatMap(List::stream) + .collect(Collectors.toList())); } - /** - * Adds {@code openKeys} to the open key table DB only, and asserts that they - * are present after the addition. Adds each key to the table with a single - * block of size {@code keySize}. - * @throws Exception - */ - private void addToOpenKeyTableDB(long keySize, OpenKeyBucket... openKeys) - throws Exception { + private void addToOpenKeyTableDB(long keySize, + List> openKeys) throws Exception { - for (OpenKeyBucket openKeyBucket: openKeys) { - String volume = openKeyBucket.getVolumeName(); - String bucket = openKeyBucket.getBucketName(); - - for (OpenKey openKey: openKeyBucket.getKeysList()) { - OmKeyInfo keyInfo = OMRequestTestUtils.createOmKeyInfo(volume, bucket, - openKey.getName(), replicationType, replicationFactor); - if (keySize > 0) { - OMRequestTestUtils.addKeyLocationInfo(keyInfo, 0, keySize); - } - OMRequestTestUtils.addKeyToTable(true, false, - keyInfo, openKey.getClientID(), 0L, omMetadataManager); + for (Pair openKey : openKeys) { + final long clientID = openKey.getLeft(); + final OmKeyInfo omKeyInfo = openKey.getRight(); + if (keySize > 0) { + OMRequestTestUtils.addKeyLocationInfo(omKeyInfo, 0, keySize); + } + if (getBucketLayout().isFileSystemOptimized()) { + OMRequestTestUtils.addFileToKeyTable( + true, false, omKeyInfo.getFileName(), + omKeyInfo, clientID, 0L, omMetadataManager); + } else { + OMRequestTestUtils.addKeyToTable( + true, false, omKeyInfo, clientID, 0L, omMetadataManager); } } - assertInOpenKeyTable(openKeys); } - /** - * Constructs a list of {@link OpenKeyBucket} objects of size {@code numKeys}. - * The keys created will all have the same volume and bucket, but - * randomized key names and client IDs. These keys are not added to the - * open key table. - * - * @param volume The volume all open keys created will have. - * @param bucket The bucket all open keys created will have. - * @param numKeys The number of keys with randomized key names and client - * IDs to create. - * @return A list of new open keys with size {@code numKeys}. + /* + * Make open keys with randomized key name and client ID */ - private OpenKeyBucket makeOpenKeys(String volume, String bucket, - int numKeys) { - return makeOpenKeys(volume, bucket, numKeys, false); + private List> makeOpenKeys( + String volume, String bucket, int count) { + return makeOpenKeys(volume, bucket, null, count); } - /** - * Constructs a list of {@link OpenKeyBucket} objects of size {@code numKeys}. - * The keys created will all have the same volume and bucket, but - * randomized key names and client IDs. These keys are not added to the - * open key table. - * - * @param volume The volume all open keys created will have. - * @param bucket The bucket all open keys created will have. - * @param numKeys The number of keys with randomized client IDs to create. - * @param fixedKeyName If set, get key name from the {@code keyName} field, - * otherwise, generate random key name. - * @return A list of new open keys with size {@code numKeys}. + /* + * Make open keys with same key name and randomized client ID */ - private OpenKeyBucket makeOpenKeys(String volume, String bucket, - int numKeys, boolean fixedKeyName) { + private List> makeOpenKeys( + String volume, String bucket, String key, int count) { + + List> keys = new ArrayList<>(count); - OpenKeyBucket.Builder keysPerBucketBuilder = - OpenKeyBucket.newBuilder() + OmKeyInfo.Builder builder = new OmKeyInfo.Builder() .setVolumeName(volume) - .setBucketName(bucket); + .setBucketName(bucket) + .setReplicationConfig(ReplicationConfig.fromTypeAndFactor( + ReplicationType.RATIS, ReplicationFactor.THREE)); - OpenKey.Builder openKeyBuilder = OpenKey.newBuilder().setName(keyName); + if (getBucketLayout().isFileSystemOptimized()) { + builder.setParentObjectID(random.nextLong()); + } - for (int i = 0; i < numKeys; i++) { - openKeyBuilder.setClientID(random.nextLong()); - if (!fixedKeyName) { - openKeyBuilder.setName(UUID.randomUUID().toString()); + for (int i = 0; i < count; i++) { + final String name = key != null ? key : UUID.randomUUID().toString(); + builder.setKeyName(name); + if (getBucketLayout().isFileSystemOptimized()) { + builder.setFileName(OzoneFSUtils.getFileName(name)); } - keysPerBucketBuilder.addKeys(openKeyBuilder.build()); + long clientID = random.nextLong(); + keys.add(Pair.of(clientID, builder.build())); } + return keys; + } - return keysPerBucketBuilder.build(); + private void assertInOpenKeyTable(List>... allKeys) + throws Exception { + + assertInOpenKeyTable(Arrays.stream(allKeys) + .flatMap(List::stream) + .collect(Collectors.toList())); } - private void assertInOpenKeyTable(OpenKeyBucket... openKeys) + private void assertInOpenKeyTable(List> openKeys) throws Exception { - for (String keyName : getFullOpenKeyNames(openKeys)) { + for (String keyName : getDBKeyNames(openKeys)) { Assert.assertTrue(omMetadataManager.getOpenKeyTable(getBucketLayout()) .isExist(keyName)); } } - private void assertNotInOpenKeyTable(OpenKeyBucket... openKeys) + private void assertNotInOpenKeyTable(List>... allKeys) throws Exception { - for (String keyName : getFullOpenKeyNames(openKeys)) { - Assert.assertFalse(omMetadataManager.getOpenKeyTable(getBucketLayout()) - .isExist(keyName)); - } + assertNotInOpenKeyTable(Arrays.stream(allKeys) + .flatMap(List::stream) + .collect(Collectors.toList())); } - /** - * Expands all the open keys represented by {@code openKeyBuckets} to their - * full - * key names as strings. - * @param openKeyBuckets - * @return - */ - private List getFullOpenKeyNames(OpenKeyBucket... openKeyBuckets) { - List fullKeyNames = new ArrayList<>(); - - for (OpenKeyBucket keysPerBucket: openKeyBuckets) { - String volume = keysPerBucket.getVolumeName(); - String bucket = keysPerBucket.getBucketName(); + private void assertNotInOpenKeyTable(List> openKeys) + throws Exception { - for (OpenKey openKey: keysPerBucket.getKeysList()) { - String fullName = omMetadataManager.getOpenKey(volume, bucket, - openKey.getName(), openKey.getClientID()); - fullKeyNames.add(fullName); - } + for (String keyName : getDBKeyNames(openKeys)) { + Assert.assertFalse(omMetadataManager.getOpenKeyTable(getBucketLayout()) + .isExist(keyName)); } + } - return fullKeyNames; + private List getDBKeyNames(List> openKeys) { + return openKeys.stream() + .map(getBucketLayout().isFileSystemOptimized() ? + p -> omMetadataManager.getOpenFileName( + p.getRight().getParentObjectID(), + p.getRight().getFileName(), + p.getLeft() + ) : + p -> omMetadataManager.getOpenKey( + p.getRight().getVolumeName(), + p.getRight().getBucketName(), + p.getRight().getKeyName(), + p.getLeft() + ) + ) + .collect(Collectors.toList()); } /** @@ -353,10 +397,23 @@ private OMRequest doPreExecute(OMRequest originalOmRequest) throws Exception { * {@code keysToDelete}. Returns an {@code OMRequest} which encapsulates this * {@code OpenKeyDeleteRequest}. */ - private OMRequest createDeleteOpenKeyRequest(OpenKeyBucket... keysToDelete) { + private OMRequest createDeleteOpenKeyRequest( + List> keysToDelete) { + + List names = getDBKeyNames(keysToDelete); + + // TODO: HDDS-6563, volume and bucket in OpenKeyBucket doesn't matter + List openKeyBuckets = names.stream() + .map(name -> OpenKeyBucket.newBuilder() + .setVolumeName("") + .setBucketName("") + .addKeys(OpenKey.newBuilder().setName(name).build()) + .build()) + .collect(Collectors.toList()); + DeleteOpenKeysRequest deleteOpenKeysRequest = DeleteOpenKeysRequest.newBuilder() - .addAllOpenKeysPerBucket(Arrays.asList(keysToDelete)) + .addAllOpenKeysPerBucket(openKeyBuckets) .build(); return OMRequest.newBuilder() diff --git a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/response/key/TestOMOpenKeysDeleteResponse.java b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/response/key/TestOMOpenKeysDeleteResponse.java index b87a9511be04..876c3bd76a99 100644 --- a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/response/key/TestOMOpenKeysDeleteResponse.java +++ b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/response/key/TestOMOpenKeysDeleteResponse.java @@ -18,15 +18,22 @@ package org.apache.hadoop.ozone.om.response.key; +import org.apache.hadoop.ozone.om.helpers.BucketLayout; +import org.apache.hadoop.ozone.om.helpers.OzoneFSUtils; import org.apache.hadoop.ozone.om.request.OMRequestTestUtils; -import org.junit.Assert; -import org.junit.Test; import org.apache.hadoop.ozone.om.helpers.OmKeyInfo; import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos; import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.Status; import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.OMResponse; +import org.junit.Assert; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.Parameterized; +import org.junit.runners.Parameterized.Parameters; +import java.util.Arrays; +import java.util.Collection; import java.util.HashMap; import java.util.Map; import java.util.UUID; @@ -34,8 +41,27 @@ /** * Tests OMOpenKeysDeleteResponse. */ +@RunWith(Parameterized.class) public class TestOMOpenKeysDeleteResponse extends TestOMKeyResponse { private static final long KEY_LENGTH = 100; + private final BucketLayout bucketLayout; + + public TestOMOpenKeysDeleteResponse(BucketLayout bucketLayout) { + this.bucketLayout = bucketLayout; + } + + @Override + public BucketLayout getBucketLayout() { + return bucketLayout; + } + + @Parameters + public static Collection bucketLayouts() { + return Arrays.asList( + BucketLayout.DEFAULT, + BucketLayout.FILE_SYSTEM_OPTIMIZED + ); + } /** * Tests deleting a subset of keys from the open key table DB when the keys @@ -129,7 +155,7 @@ private void createAndCommitResponse(Map keysToDelete, .build(); OMOpenKeysDeleteResponse response = new OMOpenKeysDeleteResponse(omResponse, - keysToDelete, true); + keysToDelete, true, getBucketLayout()); // Operations are only added to the batch by this method when status is OK. response.checkAndUpdateDB(omMetadataManager, batchOperation); @@ -165,6 +191,7 @@ private Map addOpenKeysToDB(String volume, int numKeys, String bucket = UUID.randomUUID().toString(); String key = UUID.randomUUID().toString(); long clientID = random.nextLong(); + long parentID = random.nextLong(); OmKeyInfo omKeyInfo = OMRequestTestUtils.createOmKeyInfo(volume, bucket, key, replicationType, replicationFactor); @@ -173,14 +200,23 @@ private Map addOpenKeysToDB(String volume, int numKeys, OMRequestTestUtils.addKeyLocationInfo(omKeyInfo, 0, keyLength); } - String openKey = omMetadataManager.getOpenKey(volume, bucket, - key, clientID); + final String openKey; // Add to the open key table DB, not cache. // In a real execution, the open key would have been removed from the // cache by the request, and it would only remain in the DB. - OMRequestTestUtils.addKeyToTable(true, false, omKeyInfo, - clientID, 0L, omMetadataManager); + if (getBucketLayout().isFileSystemOptimized()) { + String file = OzoneFSUtils.getFileName(key); + omKeyInfo.setFileName(file); + omKeyInfo.setParentObjectID(parentID); + OMRequestTestUtils.addFileToKeyTable(true, false, file, omKeyInfo, + clientID, 0L, omMetadataManager); + openKey = omMetadataManager.getOpenFileName(parentID, file, clientID); + } else { + OMRequestTestUtils.addKeyToTable(true, false, omKeyInfo, + clientID, 0L, omMetadataManager); + openKey = omMetadataManager.getOpenKey(volume, bucket, key, clientID); + } Assert.assertTrue(omMetadataManager.getOpenKeyTable(getBucketLayout()) .isExist(openKey));