Reorder delete table4

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.
    */

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<OmKeyInfo> 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();
+      }
+    }
+  }
 }

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));
+  }
 }
 

hadoop-ozone/integration-test/pom.xml

@@ -112,6 +112,11 @@ https://maven.apache.org/xsd/maven-4.0.0.xsd">
       <type>test-jar</type>
       <scope>test</scope>
     </dependency>
+    <dependency>
+      <groupId>org.jmockit</groupId>
+      <artifactId>jmockit</artifactId>
+      <scope>test</scope>
+    </dependency>
     <dependency>
       <groupId>junit</groupId>
       <artifactId>junit</artifactId>

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<String, ? extends TypedTable.KeyValue<String,
+            RepeatedOmKeyInfo>> it = omMetadataManager.getDeletedTable()
+                .iterator();
+        while (it.hasNext()) {
+          Table.KeyValue<String, RepeatedOmKeyInfo> v = it.next();
+          String[] split = v.getKey().split("-");
+          RepeatedOmKeyInfo deletedKeys = v.getValue();
+
+          if (expectedTimestamp.equals(split[0])) {
+            Map<String, String> 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<String, String> 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,

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(() -> {

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 {

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:
    * |----------------------------------------------------------------------|

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;

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;