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/07 16:09:07 UTC
[GitHub] [ozone] sadanand48 opened a new pull request #2128: [FSO] Improve KeyDeletingService to cleanup FSO files
sadanand48 opened a new pull request #2128:
URL: https://github.com/apache/ozone/pull/2128
## What changes were proposed in this pull request?
Added unit test to verify whether `KeyDeletingService` deletes FSO files. The DirectoryDeletingService is responsible for moving subfiles of deleted directories into the DeletedTable. The DeletedTable however expects the older key format (volume/bucket/key) . The fix in this change ensures that the right key format is written into the `DeletedTable`
## What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-5042
## How was this patch tested?
UT
--
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
[GitHub] [ozone] rakeshadr commented on a change in pull request #2128: HDDS-5042.[FSO] Improve KeyDeletingService to cleanup FSO files
Posted by GitBox <gi...@apache.org>.
rakeshadr commented on a change in pull request #2128:
URL: https://github.com/apache/ozone/pull/2128#discussion_r609490338
##########
File path: hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestDirectoryDeletingServiceWithFSOBucket.java
##########
@@ -315,4 +318,60 @@ 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();
+
+ // Just After delete
+ assertTableRowCount(deletedDirTable, 1);
+ assertTableRowCount(keyTable, 0);
+ assertTableRowCount(dirTable, 0);
+ assertTableRowCount(deletedKeyTable, 10);
+
+ // 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());
+ }
+
+
Review comment:
Also, please add `DELETED_DIR_TABLE` to the OMKeyDeleteResponseV1 class.
```
/**
* Response for DeleteKey request.
*/
@CleanupTableInfo(cleanupTables = { FILE_TABLE, DIRECTORY_TABLE, DELETED_TABLE,
DELETED_DIR_TABLE })
public class OMKeyDeleteResponseV1 extends OMKeyDeleteResponse {
```
--
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
[GitHub] [ozone] sadanand48 commented on pull request #2128: HDDS-5042.[FSO] Improve KeyDeletingService to cleanup FSO files
Posted by GitBox <gi...@apache.org>.
sadanand48 commented on pull request #2128:
URL: https://github.com/apache/ozone/pull/2128#issuecomment-815908241
Thanks @rakeshadr @linyiqun . Addressed review comments.
--
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
[GitHub] [ozone] linyiqun commented on a change in pull request #2128: HDDS-5042.[FSO] Improve KeyDeletingService to cleanup FSO files
Posted by GitBox <gi...@apache.org>.
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
[GitHub] [ozone] sadanand48 commented on a change in pull request #2128: HDDS-5042.[FSO] Improve KeyDeletingService to cleanup FSO files
Posted by GitBox <gi...@apache.org>.
sadanand48 commented on a change in pull request #2128:
URL: https://github.com/apache/ozone/pull/2128#discussion_r609560851
##########
File path: hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestDirectoryDeletingServiceWithFSOBucket.java
##########
@@ -315,4 +318,60 @@ 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();
+
+ // Just After delete
+ assertTableRowCount(deletedDirTable, 1);
+ assertTableRowCount(keyTable, 0);
+ assertTableRowCount(dirTable, 0);
+ assertTableRowCount(deletedKeyTable, 10);
+
+ // 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());
+ }
+
+
Review comment:
Done.
--
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
[GitHub] [ozone] rakeshadr commented on a change in pull request #2128: HDDS-5042.[FSO] Improve KeyDeletingService to cleanup FSO files
Posted by GitBox <gi...@apache.org>.
rakeshadr commented on a change in pull request #2128:
URL: https://github.com/apache/ozone/pull/2128#discussion_r609476023
##########
File path: hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestDirectoryDeletingServiceWithFSOBucket.java
##########
@@ -315,4 +318,60 @@ 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();
+
+ // Just After delete
+ assertTableRowCount(deletedDirTable, 1);
Review comment:
Can we remove following two assertions statements, because this could be there only for a fraction of seconds to milliseconds depends on how fast it is getting picked up by DDS. It can induce random failures in CI.
```
assertTableRowCount(deletedDirTable, 1);
assertTableRowCount(deletedKeyTable, 10);
```
Below assertion is enough to verify that DDS is doing garbage cleanup
```
assertTableRowCount(keyTable, 0);
assertTableRowCount(dirTable, 0);
```
##########
File path: hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestDirectoryDeletingServiceWithFSOBucket.java
##########
@@ -315,4 +318,60 @@ 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();
+
+ // Just After delete
+ assertTableRowCount(deletedDirTable, 1);
+ assertTableRowCount(keyTable, 0);
+ assertTableRowCount(dirTable, 0);
+ assertTableRowCount(deletedKeyTable, 10);
+
+ // 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());
+ }
+
+
Review comment:
Please add following test case to verify the direct file deletion.
@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);
}
DirectoryDeletingService dirDeletingService =
(DirectoryDeletingService) cluster.getOzoneManager().getKeyManager()
.getDirDeletingService();
KeyDeletingService keyDeletingService =
(KeyDeletingService) cluster.getOzoneManager().getKeyManager()
.getDeletingService();
// Just After delete
assertTableRowCount(keyTable, 0);
assertTableRowCount(dirTable, 1);
// Eventually keys would get cleaned up from deletedTables too
assertTableRowCount(deletedDirTable, 0);
assertTableRowCount(deletedKeyTable, 0);
assertSubPathsCount(dirDeletingService.getMovedFilesCount(), 0);
assertSubPathsCount(dirDeletingService.getDeletedDirsCount(), 0);
// verify whether KeyDeletingService has purged the keys
Assert.assertEquals(10, keyDeletingService.getDeletedKeyCount().get());
}
##########
File path: hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestDirectoryDeletingServiceWithFSOBucket.java
##########
@@ -315,4 +318,60 @@ 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();
+
+ // Just After delete
+ assertTableRowCount(deletedDirTable, 1);
+ assertTableRowCount(keyTable, 0);
+ assertTableRowCount(dirTable, 0);
+ assertTableRowCount(deletedKeyTable, 10);
+
+ // 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());
+ }
+
+
Review comment:
Inorder to run the direct file deletion, we need to do correct keyName in `OMKeyDeleteResponseV1` similar way you did in OMPathsPurgeResponseV1.java class
https://github.com/apache/ozone/blob/HDDS-2939/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/response/key/OMKeyDeleteResponseV1.java#L81
```
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);
addDeletionToBatch(omMetadataManager, batchOperation, keyTable,
deletedKey, omKeyInfo);
```
--
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
[GitHub] [ozone] sadanand48 commented on a change in pull request #2128: HDDS-5042.[FSO] Improve KeyDeletingService to cleanup FSO files
Posted by GitBox <gi...@apache.org>.
sadanand48 commented on a change in pull request #2128:
URL: https://github.com/apache/ozone/pull/2128#discussion_r609811362
##########
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:
Done.
##########
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:
Done.
--
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
[GitHub] [ozone] rakeshadr merged pull request #2128: HDDS-5042.[FSO] Improve KeyDeletingService to cleanup FSO files
Posted by GitBox <gi...@apache.org>.
rakeshadr merged pull request #2128:
URL: https://github.com/apache/ozone/pull/2128
--
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
[GitHub] [ozone] rakeshadr commented on pull request #2128: HDDS-5042.[FSO] Improve KeyDeletingService to cleanup FSO files
Posted by GitBox <gi...@apache.org>.
rakeshadr commented on pull request #2128:
URL: https://github.com/apache/ozone/pull/2128#issuecomment-816448379
Thanks @sadanand48 for the good work.
Thanks @linyiqun for the useful comments.
+1 LGTM, I will merge it shortly.
--
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
[GitHub] [ozone] sadanand48 commented on a change in pull request #2128: HDDS-5042.[FSO] Improve KeyDeletingService to cleanup FSO files
Posted by GitBox <gi...@apache.org>.
sadanand48 commented on a change in pull request #2128:
URL: https://github.com/apache/ozone/pull/2128#discussion_r609561174
##########
File path: hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestDirectoryDeletingServiceWithFSOBucket.java
##########
@@ -315,4 +318,60 @@ 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();
+
+ // Just After delete
+ assertTableRowCount(deletedDirTable, 1);
Review comment:
Done.
--
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
[GitHub] [ozone] sadanand48 commented on a change in pull request #2128: HDDS-5042.[FSO] Improve KeyDeletingService to cleanup FSO files
Posted by GitBox <gi...@apache.org>.
sadanand48 commented on a change in pull request #2128:
URL: https://github.com/apache/ozone/pull/2128#discussion_r609811978
##########
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:
Thanks for finding this 🙂
--
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