You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by GitBox <gi...@apache.org> on 2021/04/08 14:31:23 UTC

[GitHub] [ozone] linyiqun commented on a change in pull request #2128: HDDS-5042.[FSO] Improve KeyDeletingService to cleanup FSO files

linyiqun commented on a change in pull request #2128:
URL: https://github.com/apache/ozone/pull/2128#discussion_r609754880



##########
File path: hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestDirectoryDeletingServiceWithFSOBucket.java
##########
@@ -315,4 +318,112 @@ private void checkPath(Path path) {
     }
   }
 
+  @Test
+  public void testDeleteSubFiles() throws Exception {
+
+    Table<String, OmKeyInfo> deletedDirTable =
+        cluster.getOzoneManager().getMetadataManager().getDeletedDirTable();
+    Table<String, OmKeyInfo> keyTable =
+        cluster.getOzoneManager().getMetadataManager().getKeyTable();
+    Table<String, OmDirectoryInfo> dirTable =
+        cluster.getOzoneManager().getMetadataManager().getDirectoryTable();
+    Table<String, RepeatedOmKeyInfo> deletedKeyTable =
+        cluster.getOzoneManager().getMetadataManager().getDeletedTable();
+
+    Path root = new Path("/rootDir2");
+    // Create  parent dir from root.
+    fs.mkdirs(root);
+
+    // Added 10 sub files inside root dir
+    for (int i = 0; i<10; i++) {
+      Path path = new Path(root, "testKey" + i);
+      try (FSDataOutputStream stream = fs.create(path)) {
+        stream.write(1);
+      }
+    }
+
+    // Before delete
+    assertTableRowCount(deletedDirTable, 0);
+    assertTableRowCount(keyTable, 10);
+    assertTableRowCount(dirTable, 1);
+
+    fs.delete(root, true);

Review comment:
       Can we do additionally assert of the keyDeletingService.getDeletedKeyCount() before the delete operation?

##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/response/key/OMKeyDeleteResponseV1.java
##########
@@ -79,8 +81,18 @@ public void addToDBBatch(OMMetadataManager omMetadataManager,
           batchOperation, ozoneDbKey, omKeyInfo);
     } else {
       Table<String, OmKeyInfo> keyTable = omMetadataManager.getKeyTable();
+      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());
+      // For OmResponse with failure, this should do nothing. This method is
+      // not called in failure scenario in OM code.
+      keyTable.deleteWithBatch(batchOperation, ozoneDbKey);

Review comment:
        keyTable.deleteWithBatch(batchOperation, ozoneDbKey); seems repeated, this operation also done in addDeletionToBatch.
   ```java
     protected void addDeletionToBatch(
         OMMetadataManager omMetadataManager,
         BatchOperation batchOperation,
         Table<String, ?> 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);
       ...
   ```

##########
File path: hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestDirectoryDeletingServiceWithFSOBucket.java
##########
@@ -315,4 +318,112 @@ private void checkPath(Path path) {
     }
   }
 
+  @Test
+  public void testDeleteSubFiles() throws Exception {
+
+    Table<String, OmKeyInfo> deletedDirTable =
+        cluster.getOzoneManager().getMetadataManager().getDeletedDirTable();
+    Table<String, OmKeyInfo> keyTable =
+        cluster.getOzoneManager().getMetadataManager().getKeyTable();
+    Table<String, OmDirectoryInfo> dirTable =
+        cluster.getOzoneManager().getMetadataManager().getDirectoryTable();
+    Table<String, RepeatedOmKeyInfo> deletedKeyTable =
+        cluster.getOzoneManager().getMetadataManager().getDeletedTable();
+
+    Path root = new Path("/rootDir2");
+    // Create  parent dir from root.
+    fs.mkdirs(root);
+
+    // Added 10 sub files inside root dir
+    for (int i = 0; i<10; i++) {
+      Path path = new Path(root, "testKey" + i);
+      try (FSDataOutputStream stream = fs.create(path)) {
+        stream.write(1);
+      }
+    }
+
+    // Before delete
+    assertTableRowCount(deletedDirTable, 0);
+    assertTableRowCount(keyTable, 10);
+    assertTableRowCount(dirTable, 1);
+
+    fs.delete(root, true);
+
+    DirectoryDeletingService dirDeletingService =
+        (DirectoryDeletingService) cluster.getOzoneManager().getKeyManager()
+            .getDirDeletingService();
+
+    KeyDeletingService keyDeletingService =
+        (KeyDeletingService) cluster.getOzoneManager().getKeyManager()
+            .getDeletingService();
+
+    // After delete
+    assertTableRowCount(keyTable, 0);
+    assertTableRowCount(dirTable, 0);
+
+    // Eventually keys would get cleaned up from deletedTables too
+    assertTableRowCount(deletedDirTable, 0);
+    assertTableRowCount(deletedKeyTable, 0);
+
+    assertSubPathsCount(dirDeletingService.getMovedFilesCount(), 10);
+    assertSubPathsCount(dirDeletingService.getDeletedDirsCount(), 1);
+    // verify whether KeyDeletingService has purged the keys
+    Assert.assertEquals(10, keyDeletingService.getDeletedKeyCount().get());
+  }
+
+  @Test
+  public void testDeleteFiles() throws Exception {
+
+    Table<String, OmKeyInfo> deletedDirTable =
+        cluster.getOzoneManager().getMetadataManager().getDeletedDirTable();
+    Table<String, OmKeyInfo> keyTable =
+        cluster.getOzoneManager().getMetadataManager().getKeyTable();
+    Table<String, OmDirectoryInfo> dirTable =
+        cluster.getOzoneManager().getMetadataManager().getDirectoryTable();
+    Table<String, RepeatedOmKeyInfo> deletedKeyTable =
+        cluster.getOzoneManager().getMetadataManager().getDeletedTable();
+
+    Path root = new Path("/rootDir2");
+    // Create  parent dir from root.
+    fs.mkdirs(root);
+
+    // Added 10 sub files inside root dir
+    for (int i = 0; i<10; i++) {
+      Path path = new Path(root, "testKey" + i);
+      try (FSDataOutputStream stream = fs.create(path)) {
+        stream.write(1);
+      }
+    }
+
+    // Before delete
+    assertTableRowCount(deletedDirTable, 0);
+    assertTableRowCount(keyTable, 10);
+    assertTableRowCount(dirTable, 1);
+
+    for (int i = 0; i<10; i++) {
+      Path path = new Path(root, "testKey" + i);
+      fs.delete(path, true);
+    }

Review comment:
       Same comment of above.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org