-
Notifications
You must be signed in to change notification settings - Fork 588
HDDS-13034. Refactor Directory Deleting Service to use ReclaimableDirectoryFilter & ReclaimableKeyFilter #8570
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
…s and deleted subFiles Change-Id: Ic1fc709b3963cde14c2a7fb64b687322a29e642a
Change-Id: I47b24dfc3b5afa3cefbdc85ac7b3e4a9b8c94869
…ectoryFilter & ReclaimableKeyFilter Change-Id: Iffdda403cba914ddef2b75808cfbef1a72b9a2d3
Change-Id: I11acc3782aadf8393f731adcaa2a436dd9b534ae
…kets and volumes have already been deleted Change-Id: I16dc9d8f00686320b4e98fa5691420294a7f1e2f
Change-Id: Ie5fd1406bbb8af3a9ba76440dcba9b8d8db14691
Change-Id: I61ef68263ff88daa0e53dfb9d7d8eb62495d226b
Change-Id: I2667c6d12523f4dee7cbcf7c48c93803fe84d3d4
Change-Id: I5ed93af3b5ae794b0cfe4671ec2a851592edcb8c
Change-Id: I4c234b75208d96a146c7498bb6c2c188a270e1b8 # Conflicts: # hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManagerImpl.java # hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/snapshot/filter/AbstractReclaimableFilterTest.java
Change-Id: Iac3af98a7e568a135073b6704a6ad5a5fac7b427
Change-Id: I9b7b41cf667e03d48120a4201757e445227924f7
Change-Id: I2ff1cf3ecf3baa00a5c5646901f6c9ffdbe6e370
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR refactors the directory deletion service and its corresponding tests to unify snapshot directory deep cleaning and AOS directory deletion by adopting the new ReclaimableDirectoryFilter and ReclaimableKeyFilter. Key changes include replacing references to SnapshotDirectoryCleaningService with DirectoryDeletingService, refactoring deletion task logic with multi‑threaded execution and CompletableFuture handling, and deprecating and updating related configuration keys and tests.
Reviewed Changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| TestKeyDeletingService.java | Updated tests to use DirectoryDeletingService instead of SnapshotDirectoryCleaningService. |
| KeyDeletingService.java | Removed handling for InterruptedException in the deleted keys processing. |
| DirectoryDeletingService.java | Refactored deletion logic to include multi‑threading, deep-clean snapshot handling, and new filter usage. |
| AbstractKeyDeletingService.java | Adjusted API to propagate changes from refactored directory deletion logic. |
| KeyManagerImpl.java, OMDBCheckpointServlet.java, KeyManager.java | Removed SnapshotDirectoryCleaningService references and updated service instantiation. |
| Integration-test files | Updated test configuration and assertions to reflect the new unified deletion approach. |
| OMConfigKeys.java, ozone-default.xml | Marked outdated snapshot directory service configs as deprecated. |
| processedAllDeletedDirs = future.thenCombine(future, (a, b) -> a && b); | ||
| } |
Copilot
AI
Jun 6, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Combining the same future using thenCombine does not accumulate results across multiple asynchronous tasks. Consider collecting all futures into a list and using CompletableFuture.allOf() to aggregate the results for correct parallel execution handling.
| processedAllDeletedDirs = future.thenCombine(future, (a, b) -> a && b); | |
| } | |
| futures.add(future); | |
| } | |
| CompletableFuture<Void> allFutures = CompletableFuture.allOf(futures.toArray(new CompletableFuture[0])); | |
| CompletableFuture<Boolean> processedAllDeletedDirs = allFutures.thenApply(v -> | |
| futures.stream().map(CompletableFuture::join).reduce(true, (a, b) -> a && b)); |
What changes were proposed in this pull request?
With implementation of ReclaimableKeyFilter & ReclaimableDirectoryFilter both snapshot directory deep cleaning and AOS DirectoryDeletingService can be unified which will simplify the entire implementation rather than having multiple implementations for DirectoryDeletingService.
Algorithm in play:
The directory deleting service would be iterate through all the entries in the deletedDirectoryTable for a store(AOS and snapshot). One thread processing deleted directory entry for a particular store.
This thread would further initialize a DeletedDirectorySupplier which is an implementation for thread safe table iterator(We can eventually use a TableSpliterator after HDDS-12742. Make RDBStoreAbstractIterator Return Reference-Counted KeyValues #8203 gets merged).
Each thread here would be iterating through the deletedDirectoryTable independently and would submit purgeDirectoriesRequests where the logic for directoryPurge is as follows:
- Check if the deletedDirectory object from deletedDirectoryTable can be reclaimed as in check if the reference for directory object exists in the previous directory.
- If the directory exists in the previous snapshot then we need not expand the sub files inside the files[Number of files would be a lot generally (number of files >>> number of directories)] and we would just expand all the subdirectories under the directory(this would help in the recursively iterating on the sub files in the next iteration instead of traversing again from root all over). Now we cannot purge the directories as a reference exists we would just move all sub directories present in the AOS directory table and files which can be reclaimed i.e. files which are not present in the previous snapshot's fileTable by iterating through the sub file list based on deleted directory prefix in the AOS file table and move it to the deleted space of the store[Check HDDS-11244. OmPurgeDirectoriesRequest should clean up File and Directory tables of AOS for deleted snapshot directories #8509].
- If the directory doesn't exist in the previous snapshot we have to blindly remove all entries[sub files & sub directories] inside this directory irrespective whether it can be reclaimed or not and purge the directory if all the entries are going to be moved from AOS to the specific store's deleted space of the store.
Now once all threads have processed if none of the threads have been able to move any entry from the key space to deleted space(All keys already have been reclaimed in the previous run) we can go ahead update the exclusive size of the previous snapshot and the deep clean flag of the snapshot.
What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-13034
How was this patch tested?
Existing unit tests and additional assertions added on existing tests.