[ 
https://issues.apache.org/jira/browse/HDDS-6316?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Siyao Meng updated HDDS-6316:
-----------------------------
    Description: 
It looks like when a key is overwritten the old blocks aren't cleaned up by SCM.

Integration test patch on top of 
[{{ozone-1.2.0}}|https://github.com/apache/ozone/tree/ozone-1.2.0] tag:

{code|title=Integration test addition}
diff --git 
a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/client/rpc/TestSecureOzoneRpcClient.java
 
b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/client/rpc/TestSecureOzoneRpcClient.java
index 0398536a03..a6b6b34cfa 100644
--- 
a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/client/rpc/TestSecureOzoneRpcClient.java
+++ 
b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/client/rpc/TestSecureOzoneRpcClient.java
@@ -25,6 +25,8 @@
 import org.apache.hadoop.hdds.conf.OzoneConfiguration;
 import org.apache.hadoop.hdds.protocol.proto.HddsProtos;
 import org.apache.hadoop.hdds.scm.ScmConfigKeys;
+import org.apache.hadoop.hdds.scm.block.DeletedBlockLog;
+import org.apache.hadoop.hdds.scm.block.SCMBlockDeletingService;
 import org.apache.hadoop.hdds.scm.container.ContainerInfo;
 import 
org.apache.hadoop.hdds.scm.protocolPB.StorageContainerLocationProtocolClientSideTranslatorPB;
 import org.apache.hadoop.hdds.security.x509.SecurityConfig;
@@ -39,6 +41,7 @@
 import org.apache.hadoop.ozone.client.OzoneVolume;
 import org.apache.hadoop.ozone.client.io.OzoneInputStream;
 import org.apache.hadoop.ozone.client.io.OzoneOutputStream;
+import org.apache.hadoop.ozone.om.KeyDeletingService;
 import org.apache.hadoop.ozone.om.OzoneManager;
 import org.apache.hadoop.ozone.om.helpers.OmKeyArgs;
 import org.apache.hadoop.ozone.om.helpers.OmKeyInfo;
@@ -51,12 +54,16 @@
 import org.junit.BeforeClass;
 import org.junit.Ignore;
 import org.junit.Test;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+import org.slf4j.event.Level;
 
 import java.io.File;
 import java.io.IOException;
 import java.time.Instant;
 import java.util.HashMap;
 import java.util.UUID;
+import java.util.concurrent.TimeoutException;
 
 import static java.nio.charset.StandardCharsets.UTF_8;
 import static org.apache.hadoop.hdds.HddsConfigKeys.OZONE_METADATA_DIRS;
@@ -66,6 +73,9 @@
  */
 public class TestSecureOzoneRpcClient extends TestOzoneRpcClient {
 
+  private static final Logger LOG =
+      LoggerFactory.getLogger(TestSecureOzoneRpcClient.class);
+
   private static MiniOzoneCluster cluster = null;
   private static OzoneClient ozClient = null;
   private static ObjectStore store = null;
@@ -101,6 +111,8 @@ public static void init() throws Exception {
         OzoneConfigKeys.OZONE_ACL_AUTHORIZER_CLASS_NATIVE);
     CertificateClientTestImpl certificateClientTest =
         new CertificateClientTestImpl(conf);
+    GenericTestUtils.setLogLevel(SCMBlockDeletingService.LOG, Level.DEBUG);
+    GenericTestUtils.setLogLevel(KeyDeletingService.LOG, Level.DEBUG);
     cluster = MiniOzoneCluster.newBuilder(conf)
         .setNumDatanodes(10)
         .setScmId(SCM_ID)
@@ -127,6 +139,64 @@ public static void init() throws Exception {
     TestOzoneRpcClient.setClusterId(CLUSTER_ID);
   }
 
+  @Test
+  public void testOverwriteKeyExpectBlockCleanup()
+      throws IOException, InterruptedException, TimeoutException {
+    // Check if blocks are properly deleted by SCM when a key is overwritten
+    //  multiple times
+
+    String volumeName = UUID.randomUUID().toString();
+    String bucketName = UUID.randomUUID().toString();
+    Instant testStartTime = Instant.now();
+
+    String value = "sample value";
+    store.createVolume(volumeName);
+    OzoneVolume volume = store.getVolume(volumeName);
+    volume.createBucket(bucketName);
+    OzoneBucket bucket = volume.getBucket(bucketName);
+
+    for (int i = 1; i <= 2; i++) {
+      String keyName = "key00";  // + i;
+
+      try (OzoneOutputStream out = bucket.createKey(keyName,
+          value.getBytes(UTF_8).length, ReplicationType.STAND_ALONE,
+          ReplicationFactor.ONE, new HashMap<>())) {
+        out.write(value.getBytes(UTF_8));
+      }
+
+      OzoneKey key = bucket.getKey(keyName);
+      Assert.assertEquals(keyName, key.getName());
+      byte[] fileContent;
+      try(OzoneInputStream is = bucket.readKey(keyName)) {
+        fileContent = new byte[value.getBytes(UTF_8).length];
+        is.read(fileContent);
+      }
+
+      Assert.assertTrue(verifyRatisReplication(volumeName, bucketName,
+          keyName, ReplicationType.STAND_ALONE,
+          ReplicationFactor.ONE));
+      Assert.assertEquals(value, new String(fileContent, UTF_8));
+      Assert.assertFalse(key.getCreationTime().isBefore(testStartTime));
+      Assert.assertFalse(key.getModificationTime().isBefore(testStartTime));
+    }
+
+    // Wait for DeletedBlockLog to log at least one ValidTransaction
+    GenericTestUtils.waitFor(() -> {
+      final DeletedBlockLog deletedBlockLog = cluster
+          .getStorageContainerManager()
+          .getScmBlockManager()
+          .getDeletedBlockLog();
+      try {
+        int num = deletedBlockLog.getNumOfValidTransactions();
+        LOG.warn("NumOfValidTransactions: " + num);
+        return (num > 0);
+      } catch (IOException e) {
+        e.printStackTrace();
+      }
+      return false;
+    }, 500, 120000);
+  }
+
   /**
    * Tests successful completion of following operations when grpc block
    * token is used.
diff --git 
a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyDeletingService.java
 
b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyDeletingService.java
index 8be6af60bc..247e069ea3 100644
--- 
a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyDeletingService.java
+++ 
b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyDeletingService.java
@@ -66,7 +66,7 @@
  * keys.
  */
 public class KeyDeletingService extends BackgroundService {
-  private static final Logger LOG =
+  public static final Logger LOG =
       LoggerFactory.getLogger(KeyDeletingService.class);
 
   // Use only a single thread for KeyDeletion. Multiple threads would read
{code}

CC [~avijayan] [~erose]

  was:
It looks like when a key is overwritten the old blocks aren't cleaned up by SCM.

Integration test patch on top of 
[{{ozone-1.2.0}}|https://github.com/apache/ozone/tree/ozone-1.2.0] tag:

{code:diff|title=Integration test addition}
diff --git 
a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/client/rpc/TestSecureOzoneRpcClient.java
 
b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/client/rpc/TestSecureOzoneRpcClient.java
index 0398536a03..a6b6b34cfa 100644
--- 
a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/client/rpc/TestSecureOzoneRpcClient.java
+++ 
b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/client/rpc/TestSecureOzoneRpcClient.java
@@ -25,6 +25,8 @@
 import org.apache.hadoop.hdds.conf.OzoneConfiguration;
 import org.apache.hadoop.hdds.protocol.proto.HddsProtos;
 import org.apache.hadoop.hdds.scm.ScmConfigKeys;
+import org.apache.hadoop.hdds.scm.block.DeletedBlockLog;
+import org.apache.hadoop.hdds.scm.block.SCMBlockDeletingService;
 import org.apache.hadoop.hdds.scm.container.ContainerInfo;
 import 
org.apache.hadoop.hdds.scm.protocolPB.StorageContainerLocationProtocolClientSideTranslatorPB;
 import org.apache.hadoop.hdds.security.x509.SecurityConfig;
@@ -39,6 +41,7 @@
 import org.apache.hadoop.ozone.client.OzoneVolume;
 import org.apache.hadoop.ozone.client.io.OzoneInputStream;
 import org.apache.hadoop.ozone.client.io.OzoneOutputStream;
+import org.apache.hadoop.ozone.om.KeyDeletingService;
 import org.apache.hadoop.ozone.om.OzoneManager;
 import org.apache.hadoop.ozone.om.helpers.OmKeyArgs;
 import org.apache.hadoop.ozone.om.helpers.OmKeyInfo;
@@ -51,12 +54,16 @@
 import org.junit.BeforeClass;
 import org.junit.Ignore;
 import org.junit.Test;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+import org.slf4j.event.Level;
 
 import java.io.File;
 import java.io.IOException;
 import java.time.Instant;
 import java.util.HashMap;
 import java.util.UUID;
+import java.util.concurrent.TimeoutException;
 
 import static java.nio.charset.StandardCharsets.UTF_8;
 import static org.apache.hadoop.hdds.HddsConfigKeys.OZONE_METADATA_DIRS;
@@ -66,6 +73,9 @@
  */
 public class TestSecureOzoneRpcClient extends TestOzoneRpcClient {
 
+  private static final Logger LOG =
+      LoggerFactory.getLogger(TestSecureOzoneRpcClient.class);
+
   private static MiniOzoneCluster cluster = null;
   private static OzoneClient ozClient = null;
   private static ObjectStore store = null;
@@ -101,6 +111,8 @@ public static void init() throws Exception {
         OzoneConfigKeys.OZONE_ACL_AUTHORIZER_CLASS_NATIVE);
     CertificateClientTestImpl certificateClientTest =
         new CertificateClientTestImpl(conf);
+    GenericTestUtils.setLogLevel(SCMBlockDeletingService.LOG, Level.DEBUG);
+    GenericTestUtils.setLogLevel(KeyDeletingService.LOG, Level.DEBUG);
     cluster = MiniOzoneCluster.newBuilder(conf)
         .setNumDatanodes(10)
         .setScmId(SCM_ID)
@@ -127,6 +139,64 @@ public static void init() throws Exception {
     TestOzoneRpcClient.setClusterId(CLUSTER_ID);
   }
 
+  @Test
+  public void testOverwriteKeyExpectBlockCleanup()
+      throws IOException, InterruptedException, TimeoutException {
+    // Check if blocks are properly deleted by SCM when a key is overwritten
+    //  multiple times
+
+    String volumeName = UUID.randomUUID().toString();
+    String bucketName = UUID.randomUUID().toString();
+    Instant testStartTime = Instant.now();
+
+    String value = "sample value";
+    store.createVolume(volumeName);
+    OzoneVolume volume = store.getVolume(volumeName);
+    volume.createBucket(bucketName);
+    OzoneBucket bucket = volume.getBucket(bucketName);
+
+    for (int i = 1; i <= 2; i++) {
+      String keyName = "key00";  // + i;
+
+      try (OzoneOutputStream out = bucket.createKey(keyName,
+          value.getBytes(UTF_8).length, ReplicationType.STAND_ALONE,
+          ReplicationFactor.ONE, new HashMap<>())) {
+        out.write(value.getBytes(UTF_8));
+      }
+
+      OzoneKey key = bucket.getKey(keyName);
+      Assert.assertEquals(keyName, key.getName());
+      byte[] fileContent;
+      try(OzoneInputStream is = bucket.readKey(keyName)) {
+        fileContent = new byte[value.getBytes(UTF_8).length];
+        is.read(fileContent);
+      }
+
+      Assert.assertTrue(verifyRatisReplication(volumeName, bucketName,
+          keyName, ReplicationType.STAND_ALONE,
+          ReplicationFactor.ONE));
+      Assert.assertEquals(value, new String(fileContent, UTF_8));
+      Assert.assertFalse(key.getCreationTime().isBefore(testStartTime));
+      Assert.assertFalse(key.getModificationTime().isBefore(testStartTime));
+    }
+
+    // Wait for DeletedBlockLog to log at least one ValidTransaction
+    GenericTestUtils.waitFor(() -> {
+      final DeletedBlockLog deletedBlockLog = cluster
+          .getStorageContainerManager()
+          .getScmBlockManager()
+          .getDeletedBlockLog();
+      try {
+        int num = deletedBlockLog.getNumOfValidTransactions();
+        LOG.warn("NumOfValidTransactions: " + num);
+        return (num > 0);
+      } catch (IOException e) {
+        e.printStackTrace();
+      }
+      return false;
+    }, 500, 120000);
+  }
+
   /**
    * Tests successful completion of following operations when grpc block
    * token is used.
diff --git 
a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyDeletingService.java
 
b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyDeletingService.java
index 8be6af60bc..247e069ea3 100644
--- 
a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyDeletingService.java
+++ 
b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyDeletingService.java
@@ -66,7 +66,7 @@
  * keys.
  */
 public class KeyDeletingService extends BackgroundService {
-  private static final Logger LOG =
+  public static final Logger LOG =
       LoggerFactory.getLogger(KeyDeletingService.class);
 
   // Use only a single thread for KeyDeletion. Multiple threads would read
{code}

CC [~avijayan] [~erose]


> Potential orphan blocks when keys are overwritten
> -------------------------------------------------
>
>                 Key: HDDS-6316
>                 URL: https://issues.apache.org/jira/browse/HDDS-6316
>             Project: Apache Ozone
>          Issue Type: Bug
>          Components: OM, SCM
>    Affects Versions: 1.2.0
>            Reporter: Siyao Meng
>            Priority: Major
>
> It looks like when a key is overwritten the old blocks aren't cleaned up by 
> SCM.
> Integration test patch on top of 
> [{{ozone-1.2.0}}|https://github.com/apache/ozone/tree/ozone-1.2.0] tag:
> {code|title=Integration test addition}
> diff --git 
> a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/client/rpc/TestSecureOzoneRpcClient.java
>  
> b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/client/rpc/TestSecureOzoneRpcClient.java
> index 0398536a03..a6b6b34cfa 100644
> --- 
> a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/client/rpc/TestSecureOzoneRpcClient.java
> +++ 
> b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/client/rpc/TestSecureOzoneRpcClient.java
> @@ -25,6 +25,8 @@
>  import org.apache.hadoop.hdds.conf.OzoneConfiguration;
>  import org.apache.hadoop.hdds.protocol.proto.HddsProtos;
>  import org.apache.hadoop.hdds.scm.ScmConfigKeys;
> +import org.apache.hadoop.hdds.scm.block.DeletedBlockLog;
> +import org.apache.hadoop.hdds.scm.block.SCMBlockDeletingService;
>  import org.apache.hadoop.hdds.scm.container.ContainerInfo;
>  import 
> org.apache.hadoop.hdds.scm.protocolPB.StorageContainerLocationProtocolClientSideTranslatorPB;
>  import org.apache.hadoop.hdds.security.x509.SecurityConfig;
> @@ -39,6 +41,7 @@
>  import org.apache.hadoop.ozone.client.OzoneVolume;
>  import org.apache.hadoop.ozone.client.io.OzoneInputStream;
>  import org.apache.hadoop.ozone.client.io.OzoneOutputStream;
> +import org.apache.hadoop.ozone.om.KeyDeletingService;
>  import org.apache.hadoop.ozone.om.OzoneManager;
>  import org.apache.hadoop.ozone.om.helpers.OmKeyArgs;
>  import org.apache.hadoop.ozone.om.helpers.OmKeyInfo;
> @@ -51,12 +54,16 @@
>  import org.junit.BeforeClass;
>  import org.junit.Ignore;
>  import org.junit.Test;
> +import org.slf4j.Logger;
> +import org.slf4j.LoggerFactory;
> +import org.slf4j.event.Level;
>  
>  import java.io.File;
>  import java.io.IOException;
>  import java.time.Instant;
>  import java.util.HashMap;
>  import java.util.UUID;
> +import java.util.concurrent.TimeoutException;
>  
>  import static java.nio.charset.StandardCharsets.UTF_8;
>  import static org.apache.hadoop.hdds.HddsConfigKeys.OZONE_METADATA_DIRS;
> @@ -66,6 +73,9 @@
>   */
>  public class TestSecureOzoneRpcClient extends TestOzoneRpcClient {
>  
> +  private static final Logger LOG =
> +      LoggerFactory.getLogger(TestSecureOzoneRpcClient.class);
> +
>    private static MiniOzoneCluster cluster = null;
>    private static OzoneClient ozClient = null;
>    private static ObjectStore store = null;
> @@ -101,6 +111,8 @@ public static void init() throws Exception {
>          OzoneConfigKeys.OZONE_ACL_AUTHORIZER_CLASS_NATIVE);
>      CertificateClientTestImpl certificateClientTest =
>          new CertificateClientTestImpl(conf);
> +    GenericTestUtils.setLogLevel(SCMBlockDeletingService.LOG, Level.DEBUG);
> +    GenericTestUtils.setLogLevel(KeyDeletingService.LOG, Level.DEBUG);
>      cluster = MiniOzoneCluster.newBuilder(conf)
>          .setNumDatanodes(10)
>          .setScmId(SCM_ID)
> @@ -127,6 +139,64 @@ public static void init() throws Exception {
>      TestOzoneRpcClient.setClusterId(CLUSTER_ID);
>    }
>  
> +  @Test
> +  public void testOverwriteKeyExpectBlockCleanup()
> +      throws IOException, InterruptedException, TimeoutException {
> +    // Check if blocks are properly deleted by SCM when a key is overwritten
> +    //  multiple times
> +
> +    String volumeName = UUID.randomUUID().toString();
> +    String bucketName = UUID.randomUUID().toString();
> +    Instant testStartTime = Instant.now();
> +
> +    String value = "sample value";
> +    store.createVolume(volumeName);
> +    OzoneVolume volume = store.getVolume(volumeName);
> +    volume.createBucket(bucketName);
> +    OzoneBucket bucket = volume.getBucket(bucketName);
> +
> +    for (int i = 1; i <= 2; i++) {
> +      String keyName = "key00";  // + i;
> +
> +      try (OzoneOutputStream out = bucket.createKey(keyName,
> +          value.getBytes(UTF_8).length, ReplicationType.STAND_ALONE,
> +          ReplicationFactor.ONE, new HashMap<>())) {
> +        out.write(value.getBytes(UTF_8));
> +      }
> +
> +      OzoneKey key = bucket.getKey(keyName);
> +      Assert.assertEquals(keyName, key.getName());
> +      byte[] fileContent;
> +      try(OzoneInputStream is = bucket.readKey(keyName)) {
> +        fileContent = new byte[value.getBytes(UTF_8).length];
> +        is.read(fileContent);
> +      }
> +
> +      Assert.assertTrue(verifyRatisReplication(volumeName, bucketName,
> +          keyName, ReplicationType.STAND_ALONE,
> +          ReplicationFactor.ONE));
> +      Assert.assertEquals(value, new String(fileContent, UTF_8));
> +      Assert.assertFalse(key.getCreationTime().isBefore(testStartTime));
> +      Assert.assertFalse(key.getModificationTime().isBefore(testStartTime));
> +    }
> +
> +    // Wait for DeletedBlockLog to log at least one ValidTransaction
> +    GenericTestUtils.waitFor(() -> {
> +      final DeletedBlockLog deletedBlockLog = cluster
> +          .getStorageContainerManager()
> +          .getScmBlockManager()
> +          .getDeletedBlockLog();
> +      try {
> +        int num = deletedBlockLog.getNumOfValidTransactions();
> +        LOG.warn("NumOfValidTransactions: " + num);
> +        return (num > 0);
> +      } catch (IOException e) {
> +        e.printStackTrace();
> +      }
> +      return false;
> +    }, 500, 120000);
> +  }
> +
>    /**
>     * Tests successful completion of following operations when grpc block
>     * token is used.
> diff --git 
> a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyDeletingService.java
>  
> b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyDeletingService.java
> index 8be6af60bc..247e069ea3 100644
> --- 
> a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyDeletingService.java
> +++ 
> b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyDeletingService.java
> @@ -66,7 +66,7 @@
>   * keys.
>   */
>  public class KeyDeletingService extends BackgroundService {
> -  private static final Logger LOG =
> +  public static final Logger LOG =
>        LoggerFactory.getLogger(KeyDeletingService.class);
>  
>    // Use only a single thread for KeyDeletion. Multiple threads would read
> {code}
> CC [~avijayan] [~erose]



--
This message was sent by Atlassian Jira
(v8.20.1#820001)

---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to