diff --git a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/OmUtils.java b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/OmUtils.java index f6e0bd188230..49b176b4337d 100644 --- a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/OmUtils.java +++ b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/OmUtils.java @@ -518,6 +518,45 @@ public static RepeatedOmKeyInfo prepareKeyForDelete(OmKeyInfo keyInfo, return repeatedOmKeyInfo; } + /** + * Keys in deletion table are hex-encoded String of Timestamp + UpdateID. + * The idea is to guarantee rough ordering of deletion by timestamp, and + * to guarantee the uniqueness by UpdateID (transaction index). + * + * UpdateID is transaction index from Ratis, assuming it is already set + * during the OMKeyRequest. The transaction index may not be consistently + * increasing across OM restart, or Ratis configuration change. + * + * To address the ordering inconsistency, it is prefixed with hex-encoded + * timestamp obtained by Time.now(). Its precision depends on the system - + * mostly around 10 milliseconds. We only need rough ordering of actual + * deletion of deleted keys - based on the order of deletion called by + * clients. We want older deleted keys being processed earlier by deletion + * service, and newer keys being processed later. + * + * Caveat: the only change where it loses the uniqueness is when the system + * clock issues the same two timestamps for JVM across OM restart. Say, + * before restart, in time t1 from Time.now() with transaction index 1 + * "t1-0001" issued on deletion, and OM restart happens, and t2 from + * Time.now() with another transaction index 1 "t2-0001" issued on deletion + * with transaction index being reset. + * Although t1!=t2 is not guaranteed by the system, we can (almost) safely + * assume OM restart takes long enough so that t1==t2 almost never happens. + * + * @param timestamp + * @param transactionLogIndex + * @return Unique and monotonically increasing String for deletion table + */ + public static String keyForDeleteTable(long timestamp, + long transactionLogIndex) { + // Rule out default value of protocol buffers + assert timestamp != 0; + + // Log.toHexString() is much faster, but the string is compact. Heading 0s + // are all erased. e.g. 15L becomes "F", while we want "00000000000F". + return String.format("%016X-%016X", timestamp, transactionLogIndex); + } + /** * Verify volume name is a valid DNS name. */ diff --git a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/RepeatedOmKeyInfo.java b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/RepeatedOmKeyInfo.java index feb12f884a68..3a8ca35e3de4 100644 --- a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/RepeatedOmKeyInfo.java +++ b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/RepeatedOmKeyInfo.java @@ -19,6 +19,8 @@ import java.io.IOException; import java.util.ArrayList; import java.util.List; + +import org.apache.hadoop.ozone.OzoneConsts; import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos .RepeatedKeyInfo; import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos @@ -40,10 +42,14 @@ public RepeatedOmKeyInfo(List omKeyInfos) { } public RepeatedOmKeyInfo(OmKeyInfo omKeyInfos) { - this.omKeyInfoList = new ArrayList<>(); + this(); this.omKeyInfoList.add(omKeyInfos); } + public RepeatedOmKeyInfo() { + this.omKeyInfoList = new ArrayList<>(); + } + public void addOmKeyInfo(OmKeyInfo info) { this.omKeyInfoList.add(info); } @@ -103,4 +109,20 @@ public RepeatedOmKeyInfo build() { public RepeatedOmKeyInfo copyObject() { return new RepeatedOmKeyInfo(new ArrayList<>(omKeyInfoList)); } + + /** + * If this key is in a GDPR enforced bucket, then before moving + * KeyInfo to deletedTable, remove the GDPR related metadata and + * FileEncryptionInfo from KeyInfo. + */ + public void clearGDPRdata() { + for (OmKeyInfo keyInfo : omKeyInfoList) { + if (Boolean.valueOf(keyInfo.getMetadata().get(OzoneConsts.GDPR_FLAG))) { + keyInfo.getMetadata().remove(OzoneConsts.GDPR_FLAG); + keyInfo.getMetadata().remove(OzoneConsts.GDPR_ALGORITHM); + keyInfo.getMetadata().remove(OzoneConsts.GDPR_SECRET); + keyInfo.clearFileEncryptionInfo(); + } + } + } } diff --git a/hadoop-ozone/common/src/test/java/org/apache/hadoop/ozone/TestOmUtils.java b/hadoop-ozone/common/src/test/java/org/apache/hadoop/ozone/TestOmUtils.java index c2c1bb542840..f8821da86aa5 100644 --- a/hadoop-ozone/common/src/test/java/org/apache/hadoop/ozone/TestOmUtils.java +++ b/hadoop-ozone/common/src/test/java/org/apache/hadoop/ozone/TestOmUtils.java @@ -184,5 +184,23 @@ public void testGetOmHostsFromConfig() { Assert.assertTrue(getOmHostsFromConfig(conf, "newId").isEmpty()); } + + @Test + public void testKeyForDeleteTable() { + // Past updateID. First half of the key is hex-encoded timestamp + Assert.assertEquals("0000000000000001-0000000000000000", + OmUtils.keyForDeleteTable(1L, 0L)); + + // Pre-ratis updateID + Assert.assertEquals("0000000000000003-0000000000000000", + OmUtils.keyForDeleteTable(3L, 0L)); + + // Post-ratis updateID + Assert.assertEquals("0000000000000003-0000000000000001", + OmUtils.keyForDeleteTable(3L, 1L)); + + Assert.assertEquals("0000000000000002-0000000000000144", + OmUtils.keyForDeleteTable(2L, 324L)); + } } diff --git a/hadoop-ozone/integration-test/pom.xml b/hadoop-ozone/integration-test/pom.xml index 3e5fe5624503..ef2616993aef 100644 --- a/hadoop-ozone/integration-test/pom.xml +++ b/hadoop-ozone/integration-test/pom.xml @@ -112,6 +112,11 @@ https://maven.apache.org/xsd/maven-4.0.0.xsd"> test-jar test + + org.jmockit + jmockit + test + junit junit diff --git a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/client/rpc/TestOzoneAtRestEncryption.java b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/client/rpc/TestOzoneAtRestEncryption.java index 563e628f94c7..32e5e9917c34 100644 --- a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/client/rpc/TestOzoneAtRestEncryption.java +++ b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/client/rpc/TestOzoneAtRestEncryption.java @@ -47,6 +47,9 @@ import org.apache.hadoop.hdds.protocol.proto.HddsProtos; import org.apache.hadoop.hdds.scm.container.ContainerInfo; import org.apache.hadoop.hdds.scm.protocolPB.StorageContainerLocationProtocolClientSideTranslatorPB; +import org.apache.hadoop.hdds.utils.db.Table; +import org.apache.hadoop.hdds.utils.db.TableIterator; +import org.apache.hadoop.hdds.utils.db.TypedTable; import org.apache.hadoop.ozone.MiniOzoneCluster; import org.apache.hadoop.ozone.OzoneConsts; import org.apache.hadoop.ozone.client.BucketArgs; @@ -70,6 +73,7 @@ import org.apache.hadoop.ozone.om.helpers.OmMultipartUploadCompleteInfo; import org.apache.hadoop.ozone.om.helpers.RepeatedOmKeyInfo; import org.apache.hadoop.ozone.om.helpers.BucketLayout; +import org.apache.hadoop.util.Time; import org.apache.ozone.test.GenericTestUtils; import static org.apache.hadoop.hdds.HddsConfigKeys.OZONE_METADATA_DIRS; @@ -83,6 +87,7 @@ import org.junit.runner.RunWith; import org.junit.runners.Parameterized; import org.mockito.Mockito; +import mockit.Expectations; /** * This class is to test all the public facing APIs of Ozone Client. @@ -349,30 +354,51 @@ public void testKeyWithEncryptionAndGdpr() throws Exception { //As TDE is enabled, the TDE encryption details should not be null. Assert.assertNotNull(key.getFileEncryptionInfo()); + OMMetadataManager omMetadataManager = ozoneManager.getMetadataManager(); + + // To retrieve the entry in delete table, timestamp is mocked and saved + long current = Time.now(); + String expectedTimestamp = String.format("%016X", current); + + new Expectations(Time.class) { + { + Time.now(); result = current; + } + }; + //Step 3 bucket.deleteKey(key.getName()); - OMMetadataManager omMetadataManager = ozoneManager.getMetadataManager(); - String objectKey = omMetadataManager.getOzoneKey(volumeName, bucketName, - keyName); - GenericTestUtils.waitFor(() -> { try { - return omMetadataManager.getDeletedTable().isExist(objectKey); + TableIterator> it = omMetadataManager.getDeletedTable() + .iterator(); + while (it.hasNext()) { + Table.KeyValue v = it.next(); + String[] split = v.getKey().split("-"); + RepeatedOmKeyInfo deletedKeys = v.getValue(); + + if (expectedTimestamp.equals(split[0])) { + Map deletedKeyMetadata = + deletedKeys.getOmKeyInfoList().get(0).getMetadata(); + Assert.assertFalse( + deletedKeyMetadata.containsKey(OzoneConsts.GDPR_FLAG)); + Assert.assertFalse( + deletedKeyMetadata.containsKey(OzoneConsts.GDPR_SECRET)); + Assert.assertFalse( + deletedKeyMetadata.containsKey(OzoneConsts.GDPR_ALGORITHM)); + Assert.assertNull( + deletedKeys.getOmKeyInfoList().get(0).getFileEncryptionInfo()); + + return true; + } + } + return false; } catch (IOException e) { return false; } }, 500, 100000); - RepeatedOmKeyInfo deletedKeys = - omMetadataManager.getDeletedTable().get(objectKey); - Map deletedKeyMetadata = - deletedKeys.getOmKeyInfoList().get(0).getMetadata(); - Assert.assertFalse(deletedKeyMetadata.containsKey(OzoneConsts.GDPR_FLAG)); - Assert.assertFalse(deletedKeyMetadata.containsKey(OzoneConsts.GDPR_SECRET)); - Assert.assertFalse( - deletedKeyMetadata.containsKey(OzoneConsts.GDPR_ALGORITHM)); - Assert.assertNull( - deletedKeys.getOmKeyInfoList().get(0).getFileEncryptionInfo()); } private boolean verifyRatisReplication(String volumeName, String bucketName, diff --git a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestOzoneManagerHAKeyDeletion.java b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestOzoneManagerHAKeyDeletion.java index ca24d5dce754..a0deda716d43 100644 --- a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestOzoneManagerHAKeyDeletion.java +++ b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestOzoneManagerHAKeyDeletion.java @@ -67,7 +67,8 @@ public void testKeyDeletion() throws Exception { GenericTestUtils.waitFor(() -> keyDeletingService.getDeletedKeyCount().get() == 4, 10000, 120000); - // Check delete table is empty or not on all OMs. + // Check delete table is empty or not on all OMs. Waiting until all keys + // purged by deletion service; delete table will eventually become empty. getCluster().getOzoneManagersList().forEach((om) -> { try { GenericTestUtils.waitFor(() -> { diff --git a/hadoop-ozone/interface-client/src/main/proto/OmClientProtocol.proto b/hadoop-ozone/interface-client/src/main/proto/OmClientProtocol.proto index 5ee314643576..c308e6ab8cec 100644 --- a/hadoop-ozone/interface-client/src/main/proto/OmClientProtocol.proto +++ b/hadoop-ozone/interface-client/src/main/proto/OmClientProtocol.proto @@ -1111,6 +1111,9 @@ message DeleteKeyArgs { required string volumeName = 1; required string bucketName = 2; repeated string keys = 3; + // This will be set when the request is received in pre-Execute. This + // value is used in deletion service. + optional uint64 modificationTime = 4; } message DeleteKeysResponse { @@ -1175,6 +1178,9 @@ message PurgePathRequest { message DeleteOpenKeysRequest { repeated OpenKeyBucket openKeysPerBucket = 1; optional BucketLayoutProto bucketLayout = 2; + // This will be set when the request is received in pre-Execute. This + // value is used in deletion service. + optional uint64 modificationTime = 3; } message OpenKeyBucket { diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmMetadataManagerImpl.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmMetadataManagerImpl.java index c499d1e38659..7ec5c3261f01 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmMetadataManagerImpl.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmMetadataManagerImpl.java @@ -154,10 +154,12 @@ public class OmMetadataManagerImpl implements OMMetadataManager { * |----------------------------------------------------------------------| * | keyTable | /volumeName/bucketName/keyName->KeyInfo | * |----------------------------------------------------------------------| - * | deletedTable | /volumeName/bucketName/keyName->RepeatedKeyInfo | + * | deletedTable(*) | /volumeName/bucketName/keyName->RepeatedKeyInfo | + * | | (timestampHex)-(trxnIndex) -> RepeatedKeyInfo | * |----------------------------------------------------------------------| * | openKey | /volumeName/bucketName/keyName/id->KeyInfo | * |----------------------------------------------------------------------| + * (*) Either case exists; see HDDS-5905 for the latest progress. * * Prefix Tables: * |----------------------------------------------------------------------| diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyDeleteRequest.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyDeleteRequest.java index 1398c0b70796..ecc7339c5c40 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyDeleteRequest.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyDeleteRequest.java @@ -19,9 +19,11 @@ package org.apache.hadoop.ozone.om.request.key; import java.io.IOException; +import java.util.Arrays; import java.util.Map; import com.google.common.base.Optional; +import org.apache.hadoop.ozone.OmUtils; import org.apache.hadoop.ozone.om.helpers.BucketLayout; import org.apache.hadoop.ozone.om.helpers.OmBucketInfo; import org.apache.hadoop.ozone.om.ratis.utils.OzoneManagerDoubleBufferHelper; @@ -96,7 +98,7 @@ public OMRequest preExecute(OzoneManager ozoneManager) throws IOException { public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, long trxnLogIndex, OzoneManagerDoubleBufferHelper omDoubleBufferHelper) { return validateAndUpdateCache(ozoneManager, trxnLogIndex, - omDoubleBufferHelper, BucketLayout.DEFAULT); + omDoubleBufferHelper, getBucketLayout()); } public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, @@ -171,10 +173,11 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, // be used by DeleteKeyService only, not used for any client response // validation, so we don't need to add to cache. // TODO: Revisit if we need it later. - + String delKey = OmUtils.keyForDeleteTable(keyArgs.getModificationTime(), + trxnLogIndex); omClientResponse = new OMKeyDeleteResponse( omResponse.setDeleteKeyResponse(DeleteKeyResponse.newBuilder()) - .build(), omKeyInfo, ozoneManager.isRatisEnabled(), + .build(), delKey, Arrays.asList(omKeyInfo), omBucketInfo.copyObject()); result = Result.SUCCESS; diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyDeleteRequestWithFSO.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyDeleteRequestWithFSO.java index e375442c514c..4a1c1e108d74 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyDeleteRequestWithFSO.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyDeleteRequestWithFSO.java @@ -21,6 +21,7 @@ import com.google.common.base.Optional; import org.apache.hadoop.hdds.utils.db.cache.CacheKey; import org.apache.hadoop.hdds.utils.db.cache.CacheValue; +import org.apache.hadoop.ozone.OmUtils; import org.apache.hadoop.ozone.audit.AuditLogger; import org.apache.hadoop.ozone.audit.OMAction; import org.apache.hadoop.ozone.om.OMMetadataManager; @@ -164,10 +165,11 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, // be used by DeleteKeyService only, not used for any client response // validation, so we don't need to add to cache. // TODO: Revisit if we need it later. - + String delKey = OmUtils.keyForDeleteTable(keyArgs.getModificationTime(), + trxnLogIndex); omClientResponse = new OMKeyDeleteResponseWithFSO(omResponse .setDeleteKeyResponse(DeleteKeyResponse.newBuilder()).build(), - keyName, omKeyInfo, ozoneManager.isRatisEnabled(), + keyName, delKey, omKeyInfo, omBucketInfo.copyObject(), keyStatus.isDirectory(), volumeId); result = Result.SUCCESS; diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeysDeleteRequest.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeysDeleteRequest.java index c33c924757b8..fc1f2f33f5ad 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeysDeleteRequest.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeysDeleteRequest.java @@ -19,9 +19,11 @@ package org.apache.hadoop.ozone.om.request.key; import com.google.common.base.Optional; +import com.google.common.base.Preconditions; import org.apache.commons.lang3.tuple.Pair; import org.apache.hadoop.hdds.utils.db.cache.CacheKey; import org.apache.hadoop.hdds.utils.db.cache.CacheValue; +import org.apache.hadoop.ozone.OmUtils; import org.apache.hadoop.ozone.audit.AuditLogger; import org.apache.hadoop.ozone.om.OMMetadataManager; import org.apache.hadoop.ozone.om.OMMetrics; @@ -49,6 +51,7 @@ import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.OMResponse; import org.apache.hadoop.ozone.security.acl.IAccessAuthorizer; import org.apache.hadoop.ozone.security.acl.OzoneObj; +import org.apache.hadoop.util.Time; import org.jetbrains.annotations.NotNull; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -80,6 +83,24 @@ public OMKeysDeleteRequest(OMRequest omRequest, BucketLayout bucketLayout) { super(omRequest, bucketLayout); } + @Override + public OMRequest preExecute(OzoneManager ozoneManager) throws IOException { + DeleteKeysRequest deleteKeysRequest = + getOmRequest().getDeleteKeysRequest(); + Preconditions.checkNotNull(deleteKeysRequest); + + OzoneManagerProtocolProtos.DeleteKeyArgs deleteKeyArgs = + deleteKeysRequest.getDeleteKeys(); + + OzoneManagerProtocolProtos.DeleteKeyArgs.Builder newKeyArgs = + deleteKeyArgs.toBuilder().setModificationTime(Time.now()); + + return getOmRequest().toBuilder() + .setDeleteKeysRequest(deleteKeysRequest.toBuilder() + .setDeleteKeys(newKeyArgs)) + .setUserInfo(getUserIfNotExists(ozoneManager)).build(); + } + @Override @SuppressWarnings("methodlength") public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, long trxnLogIndex, OzoneManagerDoubleBufferHelper omDoubleBufferHelper) { @@ -177,10 +198,13 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, omBucketInfo.incrUsedBytes(-quotaReleased); omBucketInfo.incrUsedNamespace(-1L * omKeyInfoList.size()); + String deleteKey = OmUtils.keyForDeleteTable( + deleteKeyArgs.getModificationTime(), trxnLogIndex); + final long volumeId = omMetadataManager.getVolumeId(volumeName); omClientResponse = getOmClientResponse(ozoneManager, omKeyInfoList, dirList, omResponse, - unDeletedKeys, deleteStatus, omBucketInfo, volumeId); + unDeletedKeys, deleteStatus, omBucketInfo, volumeId, deleteKey); result = Result.SUCCESS; @@ -254,15 +278,16 @@ protected OMClientResponse getOmClientResponse(OzoneManager ozoneManager, List omKeyInfoList, List dirList, OMResponse.Builder omResponse, OzoneManagerProtocolProtos.DeleteKeyArgs.Builder unDeletedKeys, - boolean deleteStatus, OmBucketInfo omBucketInfo, long volumeId) { + boolean deleteStatus, OmBucketInfo omBucketInfo, long volumeId, + String deleteKey) { OMClientResponse omClientResponse; + omClientResponse = new OMKeysDeleteResponse(omResponse .setDeleteKeysResponse( DeleteKeysResponse.newBuilder().setStatus(deleteStatus) .setUnDeletedKeys(unDeletedKeys)) .setStatus(deleteStatus ? OK : PARTIAL_DELETE).setSuccess(deleteStatus) - .build(), omKeyInfoList, ozoneManager.isRatisEnabled(), - omBucketInfo.copyObject()); + .build(), deleteKey, omKeyInfoList, omBucketInfo.copyObject()); return omClientResponse; } 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 d2e25453414a..8c3b9f2141e6 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 @@ -18,8 +18,10 @@ package org.apache.hadoop.ozone.om.request.key; import com.google.common.base.Optional; +import com.google.common.base.Preconditions; import org.apache.hadoop.hdds.utils.db.cache.CacheKey; import org.apache.hadoop.hdds.utils.db.cache.CacheValue; +import org.apache.hadoop.ozone.OmUtils; import org.apache.hadoop.ozone.om.OMMetadataManager; import org.apache.hadoop.ozone.om.OMMetrics; import org.apache.hadoop.ozone.om.OzoneManager; @@ -33,6 +35,8 @@ import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.OpenKeyBucket; import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.OpenKey; import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.OMRequest; +import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.DeleteOpenKeysRequest; +import org.apache.hadoop.util.Time; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -59,6 +63,20 @@ public OMOpenKeysDeleteRequest(OMRequest omRequest, super(omRequest, bucketLayout); } + @Override + public OMRequest preExecute(OzoneManager ozoneManager) throws IOException { + DeleteOpenKeysRequest request = getOmRequest().getDeleteOpenKeysRequest(); + Preconditions.checkNotNull(request); + + OzoneManagerProtocolProtos.DeleteOpenKeysRequest.Builder builder = + request.toBuilder() + .setModificationTime(Time.now()); + + return getOmRequest().toBuilder() + .setDeleteOpenKeysRequest(builder.build()) + .setUserInfo(getUserIfNotExists(ozoneManager)).build(); + } + @Override public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, long trxnLogIndex, OzoneManagerDoubleBufferHelper omDoubleBufferHelper) { @@ -96,8 +114,11 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, openKeyBucket, deletedOpenKeys); } + String deleteKey = OmUtils.keyForDeleteTable( + deleteOpenKeysRequest.getModificationTime(), + trxnLogIndex); omClientResponse = new OMOpenKeysDeleteResponse(omResponse.build(), - deletedOpenKeys, ozoneManager.isRatisEnabled(), getBucketLayout()); + deleteKey, deletedOpenKeys, getBucketLayout()); result = Result.SUCCESS; } catch (IOException ex) { @@ -111,8 +132,8 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, omDoubleBufferHelper); } - processResults(omMetrics, numSubmittedOpenKeys, deletedOpenKeys.size(), - deleteOpenKeysRequest, result); + processResults(omMetrics, numSubmittedOpenKeys, + deletedOpenKeys.size(), deleteOpenKeysRequest, result); return omClientResponse; } diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OmKeysDeleteRequestWithFSO.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OmKeysDeleteRequestWithFSO.java index ce9c7f6d74ea..625ab1194b91 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OmKeysDeleteRequestWithFSO.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OmKeysDeleteRequestWithFSO.java @@ -137,14 +137,16 @@ protected OMClientResponse getOmClientResponse(OzoneManager ozoneManager, List omKeyInfoList, List dirList, OzoneManagerProtocolProtos.OMResponse.Builder omResponse, OzoneManagerProtocolProtos.DeleteKeyArgs.Builder unDeletedKeys, - boolean deleteStatus, OmBucketInfo omBucketInfo, long volumeId) { + boolean deleteStatus, OmBucketInfo omBucketInfo, long volumeId, + String deleteKey) { OMClientResponse omClientResponse; + omClientResponse = new OMKeysDeleteResponseWithFSO(omResponse .setDeleteKeysResponse( OzoneManagerProtocolProtos.DeleteKeysResponse.newBuilder() .setStatus(deleteStatus).setUnDeletedKeys(unDeletedKeys)) .setStatus(deleteStatus ? OK : PARTIAL_DELETE).setSuccess(deleteStatus) - .build(), omKeyInfoList, dirList, ozoneManager.isRatisEnabled(), + .build(), deleteKey, omKeyInfoList, dirList, omBucketInfo.copyObject(), volumeId); return omClientResponse; diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/response/key/AbstractOMKeyDeleteResponse.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/response/key/AbstractOMKeyDeleteResponse.java index 0fecb2242d66..77b1736192d0 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/response/key/AbstractOMKeyDeleteResponse.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/response/key/AbstractOMKeyDeleteResponse.java @@ -19,7 +19,6 @@ package org.apache.hadoop.ozone.om.response.key; import org.apache.hadoop.hdds.utils.db.Table; -import org.apache.hadoop.ozone.OmUtils; import org.apache.hadoop.ozone.om.OMMetadataManager; import org.apache.hadoop.ozone.om.helpers.BucketLayout; import org.apache.hadoop.ozone.om.helpers.OmKeyInfo; @@ -29,8 +28,12 @@ import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos .OMResponse; import org.apache.hadoop.hdds.utils.db.BatchOperation; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import java.io.IOException; +import java.util.List; +import java.util.stream.Collectors; import javax.annotation.Nullable; import javax.annotation.Nonnull; @@ -42,21 +45,29 @@ */ @CleanupTableInfo(cleanupTables = {DELETED_TABLE}) public abstract class AbstractOMKeyDeleteResponse extends OmKeyResponse { + private static final Logger LOG = + LoggerFactory.getLogger(AbstractOMKeyDeleteResponse.class); - private boolean isRatisEnabled; + // The key in delete table, *not* in key/file table + private String deleteKey; + private List omKeyInfoList; public AbstractOMKeyDeleteResponse( - @Nonnull OMResponse omResponse, boolean isRatisEnabled) { + @Nonnull OMResponse omResponse, @Nonnull String deleteKey, + @Nonnull List keysToDelete) { super(omResponse); - this.isRatisEnabled = isRatisEnabled; + this.deleteKey = deleteKey; + this.omKeyInfoList = keysToDelete; } public AbstractOMKeyDeleteResponse(@Nonnull OMResponse omResponse, - boolean isRatisEnabled, BucketLayout bucketLayout) { + @Nonnull String deleteKey, + @Nonnull List keysToDelete, BucketLayout bucketLayout) { super(omResponse, bucketLayout); - this.isRatisEnabled = isRatisEnabled; + this.deleteKey = deleteKey; + this.omKeyInfoList = keysToDelete; } /** @@ -69,89 +80,71 @@ public AbstractOMKeyDeleteResponse(@Nonnull OMResponse omResponse, checkStatusNotOK(); } + /** + * An abstract method to retrieve the key in key table for deletion, + * depending on the bucket layout. + */ + protected abstract String getKeyToDelete( + OMMetadataManager omMetadataManager, OmKeyInfo omKeyInfo); + /** * Adds the operation of deleting the {@code keyName omKeyInfo} pair from * {@code fromTable} to the batch operation {@code batchOperation}. The * batch operation is not committed, so no changes are persisted to disk. - * The log transaction index used will be retrieved by calling - * {@link OmKeyInfo#getUpdateID} on {@code omKeyInfo}. + * + * @param omMetadataManager + * @param batchOperation */ - protected void addDeletionToBatch( + protected void deleteFromKeyTable( OMMetadataManager omMetadataManager, - BatchOperation batchOperation, - Table fromTable, - String keyName, - OmKeyInfo omKeyInfo) throws IOException { - - // For OmResponse with failure, this should do nothing. This method is - // not called in failure scenario in OM code. - fromTable.deleteWithBatch(batchOperation, keyName); - - // If Key is not empty add this to delete table. - if (!isKeyEmpty(omKeyInfo)) { - // If a deleted key is put in the table where a key with the same - // name already exists, then the old deleted key information would be - // lost. To avoid this, first check if a key with same name exists. - // deletedTable in OM Metadata stores . - // The RepeatedOmKeyInfo is the structure that allows us to store a - // list of OmKeyInfo that can be tied to same key name. For a keyName - // if RepeatedOMKeyInfo structure is null, we create a new instance, - // if it is not null, then we simply add to the list and store this - // instance in deletedTable. - RepeatedOmKeyInfo repeatedOmKeyInfo = - omMetadataManager.getDeletedTable().get(keyName); - repeatedOmKeyInfo = OmUtils.prepareKeyForDelete( - omKeyInfo, repeatedOmKeyInfo, omKeyInfo.getUpdateID(), - isRatisEnabled); - omMetadataManager.getDeletedTable().putWithBatch( - batchOperation, keyName, repeatedOmKeyInfo); + BatchOperation batchOperation) throws IOException { + if (omKeyInfoList.isEmpty()) { + return; + } + Table keyTable = + omMetadataManager.getKeyTable(getBucketLayout()); + for (OmKeyInfo omKeyInfo : omKeyInfoList) { + String key = getKeyToDelete(omMetadataManager, omKeyInfo); + keyTable.deleteWithBatch(batchOperation, key); } } - /** - * This method is used for FSO file deletes. - * Since a common deletedTable is used ,it requires to add key in the - * full format (vol/buck/key). This method deletes the key from - * file table (which is in prefix format) and adds the fullKey - * into the deletedTable - * @param keyName (format: objectId/key) - * @param fullKeyName (format: vol/buck/key) - * @param omKeyInfo - * @throws IOException - */ - protected void addDeletionToBatch( - OMMetadataManager omMetadataManager, - BatchOperation batchOperation, - Table fromTable, - String keyName, String fullKeyName, - OmKeyInfo omKeyInfo) throws IOException { - - // For OmResponse with failure, this should do nothing. This method is - // not called in failure scenario in OM code. - fromTable.deleteWithBatch(batchOperation, keyName); - - // If Key is not empty add this to delete table. - if (!isKeyEmpty(omKeyInfo)) { - // If a deleted key is put in the table where a key with the same - // name already exists, then the old deleted key information would be - // lost. To avoid this, first check if a key with same name exists. - // deletedTable in OM Metadata stores . - // The RepeatedOmKeyInfo is the structure that allows us to store a - // list of OmKeyInfo that can be tied to same key name. For a keyName - // if RepeatedOMKeyInfo structure is null, we create a new instance, - // if it is not null, then we simply add to the list and store this - // instance in deletedTable. - RepeatedOmKeyInfo repeatedOmKeyInfo = - omMetadataManager.getDeletedTable().get(fullKeyName); - repeatedOmKeyInfo = OmUtils.prepareKeyForDelete( - omKeyInfo, repeatedOmKeyInfo, omKeyInfo.getUpdateID(), - isRatisEnabled); - omMetadataManager.getDeletedTable().putWithBatch( - batchOperation, fullKeyName, repeatedOmKeyInfo); + protected void insertToDeleteTable(OMMetadataManager omMetadataManager, + BatchOperation batchOperation) throws IOException { + String key = deleteKey; + + // Making sure that the corresponding UpdateID has no duplication (no + // implicit block leak). This check should be cheap because RocksDB + // internally maintains a Bloom filter. + if (omMetadataManager.getDeletedTable().isExist(key)) { + LOG.error("An entry already exists in delete table for key {}", key); + throw new IllegalStateException("An entry already exists in delete" + + " table: " + key); + } + + // No need to collect blocks for keys without any blocks. Filter them + // out and don't put them into delete table. + List keysToDelete = omKeyInfoList.stream().filter( + (omKeyInfo1 -> !isKeyEmpty(omKeyInfo1)) + ).collect(Collectors.toList()); + + // If all key info do not have blocks, no need to put to delete table. + if (keysToDelete.isEmpty()) { + return; } + RepeatedOmKeyInfo repeatedOmKeyInfo = new RepeatedOmKeyInfo(keysToDelete); + repeatedOmKeyInfo.clearGDPRdata(); + omMetadataManager.getDeletedTable().putWithBatch( + batchOperation, key, repeatedOmKeyInfo); } + protected List getOmKeyInfoList() { + return omKeyInfoList; + } + protected void addToOmKeyInfoList(OmKeyInfo omKeyInfo) { + omKeyInfoList.add(omKeyInfo); + } @Override public abstract void addToDBBatch(OMMetadataManager omMetadataManager, BatchOperation batchOperation) throws IOException; @@ -163,7 +156,7 @@ public abstract void addToDBBatch(OMMetadataManager omMetadataManager, * @param keyInfo * @return if empty true, else false. */ - private boolean isKeyEmpty(@Nullable OmKeyInfo keyInfo) { + protected boolean isKeyEmpty(@Nullable OmKeyInfo keyInfo) { if (keyInfo == null) { return true; } diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/response/key/OMKeyDeleteResponse.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/response/key/OMKeyDeleteResponse.java index 828f82d853c2..98f96e21eb0e 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/response/key/OMKeyDeleteResponse.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/response/key/OMKeyDeleteResponse.java @@ -18,7 +18,6 @@ package org.apache.hadoop.ozone.om.response.key; -import org.apache.hadoop.hdds.utils.db.Table; import org.apache.hadoop.ozone.om.OMMetadataManager; import org.apache.hadoop.ozone.om.helpers.BucketLayout; import org.apache.hadoop.ozone.om.helpers.OmBucketInfo; @@ -29,6 +28,7 @@ import org.apache.hadoop.hdds.utils.db.BatchOperation; import java.io.IOException; +import java.util.List; import javax.annotation.Nonnull; import static org.apache.hadoop.ozone.om.OmMetadataManagerImpl.BUCKET_TABLE; @@ -41,14 +41,14 @@ @CleanupTableInfo(cleanupTables = {KEY_TABLE, DELETED_TABLE, BUCKET_TABLE}) public class OMKeyDeleteResponse extends AbstractOMKeyDeleteResponse { - private OmKeyInfo omKeyInfo; private OmBucketInfo omBucketInfo; public OMKeyDeleteResponse(@Nonnull OMResponse omResponse, - @Nonnull OmKeyInfo omKeyInfo, boolean isRatisEnabled, + @Nonnull String deleteKey, + @Nonnull List omKeyInfoList, @Nonnull OmBucketInfo omBucketInfo) { - super(omResponse, isRatisEnabled, omBucketInfo.getBucketLayout()); - this.omKeyInfo = omKeyInfo; + super(omResponse, deleteKey, omKeyInfoList, + omBucketInfo.getBucketLayout()); this.omBucketInfo = omBucketInfo; } @@ -67,12 +67,9 @@ public void addToDBBatch(OMMetadataManager omMetadataManager, // For OmResponse with failure, this should do nothing. This method is // not called in failure scenario in OM code. - String ozoneKey = omMetadataManager.getOzoneKey(omKeyInfo.getVolumeName(), - omKeyInfo.getBucketName(), omKeyInfo.getKeyName()); - Table keyTable = - omMetadataManager.getKeyTable(getBucketLayout()); - addDeletionToBatch(omMetadataManager, batchOperation, keyTable, ozoneKey, - omKeyInfo); + + deleteFromKeyTable(omMetadataManager, batchOperation); + insertToDeleteTable(omMetadataManager, batchOperation); // update bucket usedBytes. omMetadataManager.getBucketTable().putWithBatch(batchOperation, @@ -80,8 +77,12 @@ public void addToDBBatch(OMMetadataManager omMetadataManager, omBucketInfo.getBucketName()), omBucketInfo); } - protected OmKeyInfo getOmKeyInfo() { - return omKeyInfo; + @Override + protected String getKeyToDelete(OMMetadataManager omMetadataManager, + OmKeyInfo omKeyInfo) { + return omMetadataManager.getOzoneKey( + omKeyInfo.getVolumeName(), omKeyInfo.getBucketName(), + omKeyInfo.getKeyName()); } protected OmBucketInfo getOmBucketInfo() { diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/response/key/OMKeyDeleteResponseWithFSO.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/response/key/OMKeyDeleteResponseWithFSO.java index 22f1702836a4..35d03244b716 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/response/key/OMKeyDeleteResponseWithFSO.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/response/key/OMKeyDeleteResponseWithFSO.java @@ -19,7 +19,6 @@ package org.apache.hadoop.ozone.om.response.key; import org.apache.hadoop.hdds.utils.db.BatchOperation; -import org.apache.hadoop.hdds.utils.db.Table; import org.apache.hadoop.ozone.om.OMMetadataManager; import org.apache.hadoop.ozone.om.helpers.BucketLayout; import org.apache.hadoop.ozone.om.helpers.OmBucketInfo; @@ -29,6 +28,7 @@ import javax.annotation.Nonnull; import java.io.IOException; +import java.util.Arrays; import static org.apache.hadoop.ozone.om.OmMetadataManagerImpl.BUCKET_TABLE; import static org.apache.hadoop.ozone.om.OmMetadataManagerImpl.DELETED_DIR_TABLE; @@ -43,16 +43,20 @@ DELETED_TABLE, DELETED_DIR_TABLE, BUCKET_TABLE}) public class OMKeyDeleteResponseWithFSO extends OMKeyDeleteResponse { + private OmKeyInfo omKeyInfo; // in case when it's a directory private boolean isDeleteDirectory; private String keyName; private long volumeId; @SuppressWarnings("parameternumber") public OMKeyDeleteResponseWithFSO(@Nonnull OMResponse omResponse, - @Nonnull String keyName, @Nonnull OmKeyInfo omKeyInfo, - boolean isRatisEnabled, @Nonnull OmBucketInfo omBucketInfo, + @Nonnull String keyName, @Nonnull String deleteKey, + @Nonnull OmKeyInfo omKeyInfo, @Nonnull OmBucketInfo omBucketInfo, @Nonnull boolean isDeleteDirectory, @Nonnull long volumeId) { - super(omResponse, omKeyInfo, isRatisEnabled, omBucketInfo); + super(omResponse, deleteKey, Arrays.asList(omKeyInfo), + omBucketInfo); + + this.omKeyInfo = omKeyInfo; this.keyName = keyName; this.isDeleteDirectory = isDeleteDirectory; this.volumeId = volumeId; @@ -73,31 +77,22 @@ public void addToDBBatch(OMMetadataManager omMetadataManager, // For OmResponse with failure, this should do nothing. This method is // not called in failure scenario in OM code. - String ozoneDbKey = omMetadataManager.getOzonePathKey(volumeId, - getOmBucketInfo().getObjectID(), getOmKeyInfo().getParentObjectID(), - getOmKeyInfo().getFileName()); + String ozoneDbKey = omMetadataManager.getOzonePathKey( + volumeId, getOmBucketInfo().getObjectID(), + omKeyInfo.getParentObjectID(), omKeyInfo.getFileName()); if (isDeleteDirectory) { omMetadataManager.getDirectoryTable().deleteWithBatch(batchOperation, ozoneDbKey); - OmKeyInfo omKeyInfo = getOmKeyInfo(); // Sets full absolute key name to OmKeyInfo, which is // required for moving the sub-files to KeyDeletionService. omKeyInfo.setKeyName(keyName); omMetadataManager.getDeletedDirTable().putWithBatch( batchOperation, ozoneDbKey, omKeyInfo); } else { - Table keyTable = - omMetadataManager.getKeyTable(getBucketLayout()); - OmKeyInfo omKeyInfo = getOmKeyInfo(); - // Sets full absolute key name to OmKeyInfo, which is - // required for moving the sub-files to KeyDeletionService. - omKeyInfo.setKeyName(keyName); - String deletedKey = omMetadataManager - .getOzoneKey(omKeyInfo.getVolumeName(), omKeyInfo.getBucketName(), - omKeyInfo.getKeyName()); - addDeletionToBatch(omMetadataManager, batchOperation, keyTable, - ozoneDbKey, deletedKey, omKeyInfo); + // UpdateID is already set, so clearing GDPR data is enough + deleteFromKeyTable(omMetadataManager, batchOperation); + insertToDeleteTable(omMetadataManager, batchOperation); } // update bucket usedBytes. @@ -106,6 +101,12 @@ public void addToDBBatch(OMMetadataManager omMetadataManager, getOmBucketInfo().getBucketName()), getOmBucketInfo()); } + @Override + protected String getKeyToDelete(OMMetadataManager omMetadataManager, + OmKeyInfo omKeyInfo1) { + return omKeyInfo1.getPath(); + } + @Override public BucketLayout getBucketLayout() { return BucketLayout.FILE_SYSTEM_OPTIMIZED; diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/response/key/OMKeysDeleteResponse.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/response/key/OMKeysDeleteResponse.java index edba72c0b7f9..53bf5cc148d6 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/response/key/OMKeysDeleteResponse.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/response/key/OMKeysDeleteResponse.java @@ -19,7 +19,6 @@ package org.apache.hadoop.ozone.om.response.key; import org.apache.hadoop.hdds.utils.db.BatchOperation; -import org.apache.hadoop.hdds.utils.db.Table; import org.apache.hadoop.ozone.om.OMMetadataManager; import org.apache.hadoop.ozone.om.helpers.BucketLayout; import org.apache.hadoop.ozone.om.helpers.OmBucketInfo; @@ -42,14 +41,12 @@ */ @CleanupTableInfo(cleanupTables = {KEY_TABLE, DELETED_TABLE, BUCKET_TABLE}) public class OMKeysDeleteResponse extends AbstractOMKeyDeleteResponse { - private List omKeyInfoList; private OmBucketInfo omBucketInfo; public OMKeysDeleteResponse(@Nonnull OMResponse omResponse, - @Nonnull List keyDeleteList, - boolean isRatisEnabled, @Nonnull OmBucketInfo omBucketInfo) { - super(omResponse, isRatisEnabled); - this.omKeyInfoList = keyDeleteList; + @Nonnull String deleteKey, @Nonnull List keysToDelete, + @Nonnull OmBucketInfo omBucketInfo) { + super(omResponse, deleteKey, keysToDelete); this.omBucketInfo = omBucketInfo; } @@ -74,22 +71,9 @@ public void checkAndUpdateDB(OMMetadataManager omMetadataManager, @Override public void addToDBBatch(OMMetadataManager omMetadataManager, BatchOperation batchOperation) throws IOException { - String volumeName = ""; - String bucketName = ""; - String keyName = ""; - Table keyTable = - omMetadataManager.getKeyTable(getBucketLayout()); - for (OmKeyInfo omKeyInfo : omKeyInfoList) { - volumeName = omKeyInfo.getVolumeName(); - bucketName = omKeyInfo.getBucketName(); - keyName = omKeyInfo.getKeyName(); - String deleteKey = omMetadataManager.getOzoneKey(volumeName, bucketName, - keyName); - - addDeletionToBatch(omMetadataManager, batchOperation, keyTable, - deleteKey, omKeyInfo); - } + deleteFromKeyTable(omMetadataManager, batchOperation); + insertToDeleteTable(omMetadataManager, batchOperation); // update bucket usedBytes. omMetadataManager.getBucketTable().putWithBatch(batchOperation, @@ -97,8 +81,12 @@ public void addToDBBatch(OMMetadataManager omMetadataManager, omBucketInfo.getBucketName()), omBucketInfo); } - public List getOmKeyInfoList() { - return omKeyInfoList; + @Override + protected String getKeyToDelete(OMMetadataManager omMetadataManager, + OmKeyInfo omKeyInfo) { + return omMetadataManager.getOzoneKey( + omKeyInfo.getVolumeName(), omKeyInfo.getBucketName(), + omKeyInfo.getKeyName()); } public OmBucketInfo getOmBucketInfo() { diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/response/key/OMKeysDeleteResponseWithFSO.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/response/key/OMKeysDeleteResponseWithFSO.java index d0d98e1f549c..742e41abc966 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/response/key/OMKeysDeleteResponseWithFSO.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/response/key/OMKeysDeleteResponseWithFSO.java @@ -18,7 +18,6 @@ package org.apache.hadoop.ozone.om.response.key; import org.apache.hadoop.hdds.utils.db.BatchOperation; -import org.apache.hadoop.hdds.utils.db.Table; import org.apache.hadoop.ozone.om.OMMetadataManager; import org.apache.hadoop.ozone.om.helpers.BucketLayout; import org.apache.hadoop.ozone.om.helpers.OmBucketInfo; @@ -49,10 +48,11 @@ public class OMKeysDeleteResponseWithFSO extends OMKeysDeleteResponse { public OMKeysDeleteResponseWithFSO( @NotNull OzoneManagerProtocolProtos.OMResponse omResponse, + @NotNull String deleteKey, @NotNull List keyDeleteList, - @NotNull List dirDeleteList, boolean isRatisEnabled, + @NotNull List dirDeleteList, @NotNull OmBucketInfo omBucketInfo, @Nonnull long volId) { - super(omResponse, keyDeleteList, isRatisEnabled, omBucketInfo); + super(omResponse, deleteKey, keyDeleteList, omBucketInfo); this.dirsList = dirDeleteList; this.volumeId = volId; } @@ -60,8 +60,6 @@ public OMKeysDeleteResponseWithFSO( @Override public void addToDBBatch(OMMetadataManager omMetadataManager, BatchOperation batchOperation) throws IOException { - Table keyTable = - omMetadataManager.getKeyTable(getBucketLayout()); final long bucketId = getOmBucketInfo().getObjectID(); // remove dirs from DirTable and add to DeletedDirTable @@ -75,15 +73,8 @@ public void addToDBBatch(OMMetadataManager omMetadataManager, } // remove keys from FileTable and add to DeletedTable - for (OmKeyInfo omKeyInfo : getOmKeyInfoList()) { - String ozoneDbKey = omMetadataManager.getOzonePathKey(volumeId, bucketId, - omKeyInfo.getParentObjectID(), omKeyInfo.getFileName()); - String deletedKey = omMetadataManager - .getOzoneKey(omKeyInfo.getVolumeName(), omKeyInfo.getBucketName(), - omKeyInfo.getKeyName()); - addDeletionToBatch(omMetadataManager, batchOperation, keyTable, - ozoneDbKey, deletedKey, omKeyInfo); - } + deleteFromKeyTable(omMetadataManager, batchOperation); + insertToDeleteTable(omMetadataManager, batchOperation); // update bucket usedBytes. omMetadataManager.getBucketTable().putWithBatch(batchOperation, @@ -91,6 +82,12 @@ public void addToDBBatch(OMMetadataManager omMetadataManager, getOmBucketInfo().getBucketName()), getOmBucketInfo()); } + @Override + protected String getKeyToDelete(OMMetadataManager omMetadataManager, + OmKeyInfo omKeyInfo) { + return omKeyInfo.getPath(); + } + @Override public BucketLayout getBucketLayout() { return BucketLayout.FILE_SYSTEM_OPTIMIZED; 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 d77a91bad319..29061132aea2 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 @@ -27,6 +27,7 @@ import javax.annotation.Nonnull; import java.io.IOException; +import java.util.ArrayList; import java.util.Map; import static org.apache.hadoop.ozone.om.OmMetadataManagerImpl.BUCKET_TABLE; @@ -44,11 +45,10 @@ public class OMOpenKeysDeleteResponse extends AbstractOMKeyDeleteResponse { public OMOpenKeysDeleteResponse( @Nonnull OMResponse omResponse, - @Nonnull Map keysToDelete, - boolean isRatisEnabled, + @Nonnull String deleteKey, @Nonnull Map keysToDelete, @Nonnull BucketLayout bucketLayout) { - super(omResponse, isRatisEnabled, bucketLayout); + super(omResponse, deleteKey, new ArrayList<>(), bucketLayout); this.keysToDelete = keysToDelete; } @@ -63,6 +63,14 @@ public OMOpenKeysDeleteResponse( super(omResponse, bucketLayout); } + @Override + protected String getKeyToDelete(OMMetadataManager omMetadataManager, + OmKeyInfo omKeyInfo) { + // This method will never be used, because addToDBBatch() + // is already implemented. + throw new IllegalStateException("BUG: key to delete cannot be retrieved"); + } + @Override public void addToDBBatch(OMMetadataManager omMetadataManager, BatchOperation batchOperation) throws IOException { @@ -70,9 +78,14 @@ public void addToDBBatch(OMMetadataManager omMetadataManager, Table openKeyTable = omMetadataManager.getOpenKeyTable(getBucketLayout()); - for (Map.Entry keyInfoPair : keysToDelete.entrySet()) { - addDeletionToBatch(omMetadataManager, batchOperation, openKeyTable, - keyInfoPair.getKey(), keyInfoPair.getValue()); + for (Map.Entry entry : keysToDelete.entrySet()) { + openKeyTable.deleteWithBatch(batchOperation, entry.getKey()); + if (!isKeyEmpty(entry.getValue())) { + addToOmKeyInfoList(entry.getValue()); + } + } + if (!getOmKeyInfoList().isEmpty()) { + insertToDeleteTable(omMetadataManager, batchOperation); } } } diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/service/KeyDeletingService.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/service/KeyDeletingService.java index b0c6b7a13ade..4b7f0bc64f11 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/service/KeyDeletingService.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/service/KeyDeletingService.java @@ -307,9 +307,18 @@ private void addToMap(Map, List> map, String objectKey) { // Parse volume and bucket name String[] split = objectKey.split(OM_KEY_PREFIX); - Preconditions.assertTrue(split.length > 3, "Volume and/or Bucket Name " + - "missing from Key Name."); - Pair volumeBucketPair = Pair.of(split[1], split[2]); + Pair volumeBucketPair; + + if (split.length > 3) { + // Old key formatting in delete table: /volume/bucket/key.. + // TODO: This path must be removed once HDDS-5905 completed. + volumeBucketPair = Pair.of(split[1], split[2]); + } else { + Preconditions.assertTrue(split.length == 1, + "Unknown format of delete key format"); + // For new key format in delete table, use a pair empty strings + volumeBucketPair = Pair.of("", ""); + } if (!map.containsKey(volumeBucketPair)) { map.put(volumeBucketPair, new ArrayList<>()); } diff --git a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/key/TestOMKeysDeleteRequest.java b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/key/TestOMKeysDeleteRequest.java index 4d233c59305b..a7465408768a 100644 --- a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/key/TestOMKeysDeleteRequest.java +++ b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/key/TestOMKeysDeleteRequest.java @@ -44,13 +44,20 @@ public class TestOMKeysDeleteRequest extends TestOMKeyRequest { private List deleteKeyList; private OMRequest omRequest; + @Test + public void testPreExecute() throws Exception { + createPreRequisites(); + doPreExecute(omRequest); + } + @Test public void testKeysDeleteRequest() throws Exception { createPreRequisites(); - OMKeysDeleteRequest omKeysDeleteRequest = - new OMKeysDeleteRequest(omRequest, getBucketLayout()); + OMKeysDeleteRequest omKeysDeleteRequest = new OMKeysDeleteRequest( + doPreExecute(omRequest), getBucketLayout()); + checkDeleteKeysResponse(omKeysDeleteRequest); } @@ -93,8 +100,8 @@ public void testKeysDeleteRequestFail() throws Exception { .setBucketName(bucketName).setVolumeName(volumeName) .addAllKeys(deleteKeyList).addKeys("dummy"))).build(); - OMKeysDeleteRequest omKeysDeleteRequest = - new OMKeysDeleteRequest(omRequest, getBucketLayout()); + OMKeysDeleteRequest omKeysDeleteRequest = new OMKeysDeleteRequest( + doPreExecute(omRequest), getBucketLayout()); checkDeleteKeysResponseForFailure(omKeysDeleteRequest); } @@ -158,6 +165,19 @@ protected void createPreRequisites() throws Exception { .setDeleteKeys(deleteKeyArgs).build()).build(); } + private OMRequest doPreExecute(OMRequest originalOmRequest) throws Exception { + + OMKeysDeleteRequest omKeyDeleteRequest = + new OMKeysDeleteRequest(omRequest, getBucketLayout()); + + OMRequest modifiedOmRequest = omKeyDeleteRequest.preExecute(ozoneManager); + + // Will not be equal, as UserInfo will be set. + Assert.assertNotEquals(originalOmRequest, modifiedOmRequest); + + return modifiedOmRequest; + } + public List getDeleteKeyList() { return deleteKeyList; } diff --git a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/key/TestOMKeysDeleteRequestWithFSO.java b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/key/TestOMKeysDeleteRequestWithFSO.java index e3545487e64a..b7d54de7759c 100644 --- a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/key/TestOMKeysDeleteRequestWithFSO.java +++ b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/key/TestOMKeysDeleteRequestWithFSO.java @@ -22,7 +22,9 @@ import org.apache.hadoop.ozone.om.helpers.OmKeyInfo; import org.apache.hadoop.ozone.om.request.OMRequestTestUtils; import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos; +import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.OMRequest; import org.apache.hadoop.util.Time; +import org.junit.Assert; import java.util.ArrayList; import java.util.UUID; @@ -39,7 +41,7 @@ public void testKeysDeleteRequest() throws Exception { createPreRequisites(); OmKeysDeleteRequestWithFSO omKeysDeleteRequest = - new OmKeysDeleteRequestWithFSO(getOmRequest(), + new OmKeysDeleteRequestWithFSO(doPreExecute(getOmRequest()), getBucketLayout()); checkDeleteKeysResponse(omKeysDeleteRequest); @@ -55,7 +57,7 @@ public void testKeysDeleteRequestFail() throws Exception { .addAllKeys(getDeleteKeyList()).addKeys("dummy"))).build()); OmKeysDeleteRequestWithFSO omKeysDeleteRequest = - new OmKeysDeleteRequestWithFSO(getOmRequest(), + new OmKeysDeleteRequestWithFSO(doPreExecute(getOmRequest()), getBucketLayout()); checkDeleteKeysResponseForFailure(omKeysDeleteRequest); } @@ -102,6 +104,19 @@ protected void createPreRequisites() throws Exception { .setDeleteKeys(deleteKeyArgs).build()).build()); } + private OMRequest doPreExecute(OMRequest originalOmRequest) throws Exception { + + OmKeysDeleteRequestWithFSO omKeyDeleteRequest = + new OmKeysDeleteRequestWithFSO(getOmRequest(), getBucketLayout()); + + OMRequest modifiedOmRequest = omKeyDeleteRequest.preExecute(ozoneManager); + + // Will not be equal, as UserInfo will be set. + Assert.assertNotEquals(originalOmRequest, modifiedOmRequest); + + return modifiedOmRequest; + } + @Override public BucketLayout getBucketLayout() { return BucketLayout.FILE_SYSTEM_OPTIMIZED; diff --git a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/response/TestOMResponseUtils.java b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/response/TestOMResponseUtils.java index 01f8baa99914..c7752a3a8ec8 100644 --- a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/response/TestOMResponseUtils.java +++ b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/response/TestOMResponseUtils.java @@ -37,5 +37,4 @@ public static OmBucketInfo createBucket(String volume, String bucket) { "key1", "value1").build(); } - } diff --git a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/response/key/TestOMKeyDeleteResponse.java b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/response/key/TestOMKeyDeleteResponse.java index adb9151b233d..56c269e34792 100644 --- a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/response/key/TestOMKeyDeleteResponse.java +++ b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/response/key/TestOMKeyDeleteResponse.java @@ -22,10 +22,14 @@ import org.apache.hadoop.hdds.client.RatisReplicationConfig; import org.apache.hadoop.hdds.scm.pipeline.Pipeline; import org.apache.hadoop.hdds.scm.pipeline.PipelineID; + +import org.apache.hadoop.ozone.OmUtils; import org.apache.hadoop.ozone.om.helpers.BucketLayout; import org.apache.hadoop.ozone.om.helpers.OmBucketInfo; import org.apache.hadoop.ozone.om.helpers.OmKeyLocationInfo; +import org.apache.hadoop.ozone.om.helpers.RepeatedOmKeyInfo; import org.apache.hadoop.ozone.om.request.OMRequestTestUtils; +import org.apache.hadoop.util.Time; import org.junit.Assert; import org.junit.Test; @@ -33,6 +37,7 @@ import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos; import java.util.ArrayList; +import java.util.Arrays; import java.util.List; /** @@ -42,10 +47,16 @@ public class TestOMKeyDeleteResponse extends TestOMKeyResponse { @Test public void testAddToDBBatch() throws Exception { + String ozoneKey = addKeyToTable(); OmKeyInfo omKeyInfo = omMetadataManager .getKeyTable(getBucketLayout()).get(ozoneKey); + long trxnLogIndex = 0x360L; + omBucketInfo = OmBucketInfo.newBuilder() + .setVolumeName(volumeName).setBucketName(bucketName) + .setCreationTime(Time.now()).build(); + OzoneManagerProtocolProtos.OMResponse omResponse = OzoneManagerProtocolProtos.OMResponse.newBuilder().setDeleteKeyResponse( OzoneManagerProtocolProtos.DeleteKeyResponse.getDefaultInstance()) @@ -53,8 +64,10 @@ public void testAddToDBBatch() throws Exception { .setCmdType(OzoneManagerProtocolProtos.Type.DeleteKey) .build(); + String deleteKey = OmUtils.keyForDeleteTable(Time.now(), trxnLogIndex); + OMKeyDeleteResponse omKeyDeleteResponse = getOmKeyDeleteResponse(omKeyInfo, - omResponse); + omResponse, deleteKey); Assert.assertTrue( omMetadataManager.getKeyTable(getBucketLayout()).isExist(ozoneKey)); @@ -66,22 +79,24 @@ public void testAddToDBBatch() throws Exception { Assert.assertFalse( omMetadataManager.getKeyTable(getBucketLayout()).isExist(ozoneKey)); - String deletedKey = omMetadataManager.getOzoneKey(volumeName, bucketName, - keyName); - // As default key entry does not have any blocks, it should not be in // deletedKeyTable. - Assert.assertFalse(omMetadataManager.getDeletedTable().isExist( - deletedKey)); + Assert.assertFalse(omMetadataManager.getDeletedTable().isExist(deleteKey)); } @Test public void testAddToDBBatchWithNonEmptyBlocks() throws Exception { + final String ozoneKey = addKeyToTable(); final OmKeyInfo omKeyInfo = omMetadataManager .getKeyTable(getBucketLayout()) .get(ozoneKey); + long trxnLogIndex = 0x360L; + omBucketInfo = OmBucketInfo.newBuilder() + .setVolumeName(volumeName).setBucketName(bucketName) + .setCreationTime(Time.now()).build(); + // Add block to key. List omKeyLocationInfoList = new ArrayList<>(); @@ -110,8 +125,9 @@ public void testAddToDBBatchWithNonEmptyBlocks() throws Exception { .setCmdType(OzoneManagerProtocolProtos.Type.DeleteKey) .build(); + String deleteKey = OmUtils.keyForDeleteTable(Time.now(), trxnLogIndex); OMKeyDeleteResponse omKeyDeleteResponse = getOmKeyDeleteResponse(omKeyInfo, - omResponse); + omResponse, deleteKey); Assert.assertTrue( omMetadataManager.getKeyTable(getBucketLayout()).isExist(ozoneKey)); @@ -123,16 +139,19 @@ public void testAddToDBBatchWithNonEmptyBlocks() throws Exception { Assert.assertFalse( omMetadataManager.getKeyTable(getBucketLayout()).isExist(ozoneKey)); - String deletedKey = omMetadataManager.getOzoneKey(volumeName, bucketName, - keyName); - - // Key has blocks, it should not be in deletedKeyTable. - Assert.assertTrue(omMetadataManager.getDeletedTable().isExist(deletedKey)); + // Key has blocks, it should be in deletedKeyTable. + Assert.assertTrue(omMetadataManager.getDeletedTable().isExist(deleteKey)); } @Test public void testAddToDBBatchWithErrorResponse() throws Exception { + + long trxnLogIndex = 0x360L; + omBucketInfo = OmBucketInfo.newBuilder() + .setVolumeName(volumeName).setBucketName(bucketName) + .setCreationTime(Time.now()).build(); + OmKeyInfo omKeyInfo = getOmKeyInfo(); OzoneManagerProtocolProtos.OMResponse omResponse = @@ -142,8 +161,9 @@ public void testAddToDBBatchWithErrorResponse() throws Exception { .setCmdType(OzoneManagerProtocolProtos.Type.DeleteKey) .build(); + String deleteKey = OmUtils.keyForDeleteTable(Time.now(), trxnLogIndex); OMKeyDeleteResponse omKeyDeleteResponse = getOmKeyDeleteResponse(omKeyInfo, - omResponse); + omResponse, deleteKey); String ozoneKey = addKeyToTable(); @@ -159,6 +179,7 @@ public void testAddToDBBatchWithErrorResponse() throws Exception { // keyTable. Assert.assertTrue( omMetadataManager.getKeyTable(getBucketLayout()).isExist(ozoneKey)); + Assert.assertFalse(omMetadataManager.getDeletedTable().isExist(deleteKey)); } @@ -172,8 +193,10 @@ protected String addKeyToTable() throws Exception { } protected OMKeyDeleteResponse getOmKeyDeleteResponse(OmKeyInfo omKeyInfo, - OzoneManagerProtocolProtos.OMResponse omResponse) throws Exception { - return new OMKeyDeleteResponse(omResponse, omKeyInfo, true, omBucketInfo); + OzoneManagerProtocolProtos.OMResponse omResponse, + String deleteKey) throws Exception { + return new OMKeyDeleteResponse(omResponse, deleteKey, Arrays.asList(omKeyInfo), + omBucketInfo); } protected OmBucketInfo getOmBucketInfo() { diff --git a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/response/key/TestOMKeyDeleteResponseWithFSO.java b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/response/key/TestOMKeyDeleteResponseWithFSO.java index 19eae28eb89d..fb766dafd69f 100644 --- a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/response/key/TestOMKeyDeleteResponseWithFSO.java +++ b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/response/key/TestOMKeyDeleteResponseWithFSO.java @@ -33,9 +33,10 @@ public class TestOMKeyDeleteResponseWithFSO extends TestOMKeyDeleteResponse { @Override protected OMKeyDeleteResponse getOmKeyDeleteResponse(OmKeyInfo omKeyInfo, - OzoneManagerProtocolProtos.OMResponse omResponse) throws Exception { + OzoneManagerProtocolProtos.OMResponse omResponse, + String deleteKey) throws Exception { return new OMKeyDeleteResponseWithFSO(omResponse, omKeyInfo.getKeyName(), - omKeyInfo, true, getOmBucketInfo(), false, + deleteKey, omKeyInfo, getOmBucketInfo(), false, omMetadataManager.getVolumeId(volumeName)); } diff --git a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/response/key/TestOMKeysDeleteResponse.java b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/response/key/TestOMKeysDeleteResponse.java index 94de269fbd0f..e143e586cb78 100644 --- a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/response/key/TestOMKeysDeleteResponse.java +++ b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/response/key/TestOMKeysDeleteResponse.java @@ -18,6 +18,7 @@ package org.apache.hadoop.ozone.om.response.key; +import org.apache.hadoop.ozone.OmUtils; import org.apache.hadoop.ozone.om.helpers.OmBucketInfo; import org.apache.hadoop.ozone.om.helpers.OmKeyInfo; import org.apache.hadoop.ozone.om.helpers.RepeatedOmKeyInfo; @@ -25,6 +26,7 @@ import org.apache.hadoop.ozone.om.response.OMClientResponse; import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.DeleteKeysResponse; import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.OMResponse; +import org.apache.hadoop.util.Time; import org.junit.Assert; import org.junit.Test; @@ -81,8 +83,13 @@ public void testKeysDeleteResponse() throws Exception { .setDeleteKeysResponse(DeleteKeysResponse.newBuilder() .setStatus(true)).build(); - OMClientResponse omKeysDeleteResponse = - getOmKeysDeleteResponse(omResponse, omBucketInfo); + OmBucketInfo omBucketInfo = OmBucketInfo.newBuilder() + .setVolumeName(volumeName).setBucketName(bucketName) + .setCreationTime(Time.now()).build(); + + String deleteKey = OmUtils.keyForDeleteTable(Time.now(), 360L); + OMClientResponse omKeysDeleteResponse = new OMKeysDeleteResponse( + omResponse, deleteKey, omKeyInfoList, omBucketInfo); omKeysDeleteResponse.checkAndUpdateDB(omMetadataManager, batchOperation); @@ -90,20 +97,19 @@ public void testKeysDeleteResponse() throws Exception { for (String ozKey : ozoneKeys) { Assert.assertNull( omMetadataManager.getKeyTable(getBucketLayout()).get(ozKey)); - - // ozKey had no block information associated with it, so it should have - // been removed from the key table but not added to the delete table. - RepeatedOmKeyInfo repeatedOmKeyInfo = - omMetadataManager.getDeletedTable().get(ozKey); - Assert.assertNull(repeatedOmKeyInfo); } + // deleteKey had no block information associated with it, so it should have + // been removed from the key table but not added to the delete table. + Assert.assertFalse(omMetadataManager.getDeletedTable().isExist(deleteKey)); } protected OMClientResponse getOmKeysDeleteResponse(OMResponse omResponse, OmBucketInfo omBucketInfo) { - return new OMKeysDeleteResponse( - omResponse, omKeyInfoList, true, omBucketInfo); + String deleteKey = OmUtils.keyForDeleteTable(Time.now(), 360L); + + return new OMKeysDeleteResponse(omResponse, deleteKey, + omKeyInfoList, omBucketInfo); } @Test @@ -116,20 +122,22 @@ public void testKeysDeleteResponseFail() throws Exception { .setDeleteKeysResponse(DeleteKeysResponse.newBuilder() .setStatus(false)).build(); - OMClientResponse omKeysDeleteResponse - = getOmKeysDeleteResponse(omResponse, omBucketInfo); + OmBucketInfo omBucketInfo = OmBucketInfo.newBuilder() + .setVolumeName(volumeName).setBucketName(bucketName) + .setCreationTime(Time.now()).build(); + + String deleteKey = OmUtils.keyForDeleteTable(Time.now(), 360L); + OMClientResponse omKeysDeleteResponse = new OMKeysDeleteResponse( + omResponse, deleteKey,omKeyInfoList, omBucketInfo); omKeysDeleteResponse.checkAndUpdateDB(omMetadataManager, batchOperation); for (String ozKey : ozoneKeys) { Assert.assertNotNull( omMetadataManager.getKeyTable(getBucketLayout()).get(ozKey)); - - RepeatedOmKeyInfo repeatedOmKeyInfo = - omMetadataManager.getDeletedTable().get(ozKey); - Assert.assertNull(repeatedOmKeyInfo); - } + // On failure, deletion (insert to delete table) should not be committed. + Assert.assertFalse(omMetadataManager.getDeletedTable().isExist(deleteKey)); } } diff --git a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/response/key/TestOMKeysDeleteResponseWithFSO.java b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/response/key/TestOMKeysDeleteResponseWithFSO.java index 9d17220631ab..20ca5ca25a6f 100644 --- a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/response/key/TestOMKeysDeleteResponseWithFSO.java +++ b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/response/key/TestOMKeysDeleteResponseWithFSO.java @@ -22,6 +22,7 @@ import org.apache.hadoop.hdds.protocol.proto.HddsProtos; import org.apache.hadoop.hdds.utils.db.cache.CacheKey; import org.apache.hadoop.hdds.utils.db.cache.CacheValue; +import org.apache.hadoop.ozone.OmUtils; import org.apache.hadoop.ozone.om.helpers.BucketLayout; import org.apache.hadoop.ozone.om.helpers.OmBucketInfo; import org.apache.hadoop.ozone.om.helpers.OmDirectoryInfo; @@ -105,8 +106,10 @@ protected void createPreRequisities() throws Exception { @Override protected OMClientResponse getOmKeysDeleteResponse(OMResponse omResponse, OmBucketInfo omBucketInfo) { + String deleteKey = OmUtils.keyForDeleteTable(Time.now(), 360L); + return new OMKeysDeleteResponseWithFSO( - omResponse, getOmKeyInfoList(), dirDeleteList, true, omBucketInfo, + omResponse, deleteKey, getOmKeyInfoList(), dirDeleteList, omBucketInfo, volId); } 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 510ac2afc69d..829d47922bf6 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,14 +18,18 @@ package org.apache.hadoop.ozone.om.response.key; +import org.apache.hadoop.ozone.OmUtils; import org.apache.hadoop.ozone.om.helpers.BucketLayout; import org.apache.hadoop.ozone.om.helpers.OzoneFSUtils; +import org.apache.hadoop.ozone.om.helpers.OmKeyInfo; +import org.apache.hadoop.ozone.om.helpers.RepeatedOmKeyInfo; import org.apache.hadoop.ozone.om.request.OMRequestTestUtils; -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.apache.hadoop.util.Time; + import org.junit.Assert; import org.junit.Test; import org.junit.runner.RunWith; @@ -76,22 +80,30 @@ public void testAddToDBBatchWithEmptyBlocks() throws Exception { Map keysToDelete = addOpenKeysToDB(volumeName, 3); Map keysToKeep = addOpenKeysToDB(volumeName, 3); - createAndCommitResponse(keysToDelete, Status.OK); + String delKey = OmUtils.keyForDeleteTable(Time.now(), 360L); - for (String key: keysToDelete.keySet()) { + createAndCommitResponse(keysToDelete, delKey, Status.OK); + + RepeatedOmKeyInfo deletedKeyInfos = + omMetadataManager.getDeletedTable().get("0"); + // Nothing should be in delete table, as all blocks are empty. + Assert.assertNull(deletedKeyInfos); + + for (Map.Entry entry: keysToDelete.entrySet()) { // open keys with no associated block data should have been removed - // from the open key table, but not added to the deleted table. - Assert.assertFalse( - omMetadataManager.getOpenKeyTable(getBucketLayout()).isExist(key)); - Assert.assertFalse(omMetadataManager.getDeletedTable().isExist(key)); + // from the open key table. + Assert.assertFalse(omMetadataManager.getOpenKeyTable(getBucketLayout()) + .isExist(entry.getKey())); } - for (String key: keysToKeep.keySet()) { + for (Map.Entry entry : keysToKeep.entrySet()) { // These keys should not have been removed from the open key table. - Assert.assertTrue( - omMetadataManager.getOpenKeyTable(getBucketLayout()).isExist(key)); - Assert.assertFalse(omMetadataManager.getDeletedTable().isExist(key)); + Assert.assertTrue(omMetadataManager.getOpenKeyTable(getBucketLayout()) + .isExist(entry.getKey())); } + + // Empty blocks won't be written to delete table + Assert.assertFalse(omMetadataManager.getDeletedTable().isExist(delKey)); } /** @@ -107,22 +119,46 @@ public void testAddToDBBatchWithNonEmptyBlocks() throws Exception { Map keysToKeep = addOpenKeysToDB(volumeName, 3, KEY_LENGTH); - createAndCommitResponse(keysToDelete, Status.OK); + String delKey = OmUtils.keyForDeleteTable(Time.now(), 360L); + createAndCommitResponse(keysToDelete, delKey, Status.OK); - for (String key: keysToDelete.keySet()) { + // UpdateID in keysToDelete is zero by default. Timestamp is the value + // set in the mock above. + RepeatedOmKeyInfo deletedKeyInfos = + omMetadataManager.getDeletedTable().get(delKey); + Assert.assertNotNull(deletedKeyInfos); + Assert.assertEquals(keysToDelete.size(), + deletedKeyInfos.getOmKeyInfoList().size()); + + for (Map.Entry entry: keysToDelete.entrySet()) { // These keys should have been moved from the open key table to the // delete table. - Assert.assertFalse( - omMetadataManager.getOpenKeyTable(getBucketLayout()).isExist(key)); - Assert.assertTrue(omMetadataManager.getDeletedTable().isExist(key)); + Assert.assertFalse(omMetadataManager.getOpenKeyTable(getBucketLayout()) + .isExist(entry.getKey())); + Assert.assertTrue(containsKey(deletedKeyInfos, entry.getValue())); } - for (String key: keysToKeep.keySet()) { + for (Map.Entry entry : keysToKeep.entrySet()) { // These keys should not have been moved out of the open key table. - Assert.assertTrue( - omMetadataManager.getOpenKeyTable(getBucketLayout()).isExist(key)); - Assert.assertFalse(omMetadataManager.getDeletedTable().isExist(key)); + Assert.assertTrue(omMetadataManager.getOpenKeyTable(getBucketLayout()) + .isExist(entry.getKey())); + Assert.assertFalse(containsKey(deletedKeyInfos, entry.getValue())); + } + + // Deletion (insert to delete table) should be committed as well. + Assert.assertTrue(omMetadataManager.getDeletedTable().isExist(delKey)); + } + + public boolean containsKey(RepeatedOmKeyInfo repeatedOmKeyInfo, + OmKeyInfo omKeyInfo) { + for (OmKeyInfo omKeyInfo1 : repeatedOmKeyInfo.getOmKeyInfoList()) { + if (omKeyInfo.getVolumeName().equals(omKeyInfo1.getVolumeName()) && + omKeyInfo.getBucketName().equals(omKeyInfo1.getBucketName()) && + omKeyInfo.getKeyName().equals(omKeyInfo1.getKeyName())) { + return true; + } } + return false; } /** @@ -136,15 +172,23 @@ public void testAddToDBBatchWithErrorResponse() throws Exception { omMetadataManager, getBucketLayout()); Map keysToDelete = addOpenKeysToDB(volumeName, 3); - createAndCommitResponse(keysToDelete, Status.INTERNAL_ERROR); + String delKey = OmUtils.keyForDeleteTable(Time.now(), 360L); + + createAndCommitResponse(keysToDelete, delKey, Status.INTERNAL_ERROR); + + RepeatedOmKeyInfo deletedKeyInfos = + omMetadataManager.getDeletedTable().get(delKey); + Assert.assertNull(deletedKeyInfos); for (String key: keysToDelete.keySet()) { // If an error occurs in the response, the batch operation moving keys // from the open key table to the delete table should not be committed. Assert.assertTrue( omMetadataManager.getOpenKeyTable(getBucketLayout()).isExist(key)); - Assert.assertFalse(omMetadataManager.getDeletedTable().isExist(key)); } + + // Deletion (insert to delete table) should not be committed as well. + Assert.assertFalse(omMetadataManager.getDeletedTable().isExist(delKey)); } /** @@ -155,7 +199,7 @@ public void testAddToDBBatchWithErrorResponse() throws Exception { * @throws Exception */ private void createAndCommitResponse(Map keysToDelete, - Status status) throws Exception { + String deleteKey, Status status) throws Exception { OMResponse omResponse = OMResponse.newBuilder() .setStatus(status) @@ -163,7 +207,7 @@ private void createAndCommitResponse(Map keysToDelete, .build(); OMOpenKeysDeleteResponse response = new OMOpenKeysDeleteResponse(omResponse, - keysToDelete, true, getBucketLayout()); + deleteKey, keysToDelete, getBucketLayout()); // Operations are only added to the batch by this method when status is OK. response.checkAndUpdateDB(omMetadataManager, batchOperation);