jojochuang commented on code in PR #8546:
URL: https://github.com/apache/ozone/pull/8546#discussion_r2131561575
##########
hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/OMConfigKeys.java:
##########
@@ -387,12 +387,16 @@ public final class OMConfigKeys {
*/
public static final String OZONE_SNAPSHOT_DEEP_CLEANING_ENABLED =
"ozone.snapshot.deep.cleaning.enabled";
public static final boolean OZONE_SNAPSHOT_DEEP_CLEANING_ENABLED_DEFAULT =
false;
+ @Deprecated
public static final String OZONE_SNAPSHOT_DIRECTORY_SERVICE_INTERVAL =
"ozone.snapshot.directory.service.interval";
+ @Deprecated
public static final String OZONE_SNAPSHOT_DIRECTORY_SERVICE_INTERVAL_DEFAULT
= "24h";
+ @Deprecated
public static final String OZONE_SNAPSHOT_DIRECTORY_SERVICE_TIMEOUT =
"ozone.snapshot.directory.service.timeout";
+ @Deprecated
Review Comment:
no longer used.
##########
hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/OMConfigKeys.java:
##########
@@ -387,12 +387,16 @@ public final class OMConfigKeys {
*/
public static final String OZONE_SNAPSHOT_DEEP_CLEANING_ENABLED =
"ozone.snapshot.deep.cleaning.enabled";
public static final boolean OZONE_SNAPSHOT_DEEP_CLEANING_ENABLED_DEFAULT =
false;
+ @Deprecated
Review Comment:
no longer used.
##########
hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/OMConfigKeys.java:
##########
@@ -387,12 +387,16 @@ public final class OMConfigKeys {
*/
public static final String OZONE_SNAPSHOT_DEEP_CLEANING_ENABLED =
"ozone.snapshot.deep.cleaning.enabled";
public static final boolean OZONE_SNAPSHOT_DEEP_CLEANING_ENABLED_DEFAULT =
false;
+ @Deprecated
public static final String OZONE_SNAPSHOT_DIRECTORY_SERVICE_INTERVAL =
"ozone.snapshot.directory.service.interval";
+ @Deprecated
Review Comment:
no longer used.
##########
hadoop-hdds/common/src/main/resources/ozone-default.xml:
##########
@@ -3795,9 +3795,9 @@
<property>
<name>ozone.snapshot.directory.service.interval</name>
<value>24h</value>
- <tag>OZONE, PERFORMANCE, OM</tag>
+ <tag>OZONE, PERFORMANCE, OM, DEPRECATED</tag>
<description>
- The time interval between successive SnapshotDirectoryCleaningService
+ DEPRECATED. The time interval between successive
SnapshotDirectoryCleaningService
Review Comment:
Deprecated because it is replaced by DirectoryDeletingService
##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestDirectoryDeletingServiceWithFSO.java:
##########
@@ -749,30 +748,29 @@ public void testDirDeletedTableCleanUpForSnapshot()
throws Exception {
assertSubPathsCount(dirDeletingService::getMovedFilesCount, 0);
assertSubPathsCount(dirDeletingService::getMovedDirsCount, 0);
assertSubPathsCount(dirDeletingService::getDeletedDirsCount, 0);
-
// Case-2) Delete dir
fs.delete(root, true);
// After delete. 5 sub files are still in keyTable.
// 4 dirs in dirTable.
Review Comment:
```suggestion
// 0 dirs in dirTable.
```
##########
hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/OMConfigKeys.java:
##########
@@ -387,12 +387,16 @@ public final class OMConfigKeys {
*/
public static final String OZONE_SNAPSHOT_DEEP_CLEANING_ENABLED =
"ozone.snapshot.deep.cleaning.enabled";
public static final boolean OZONE_SNAPSHOT_DEEP_CLEANING_ENABLED_DEFAULT =
false;
+ @Deprecated
public static final String OZONE_SNAPSHOT_DIRECTORY_SERVICE_INTERVAL =
"ozone.snapshot.directory.service.interval";
+ @Deprecated
public static final String OZONE_SNAPSHOT_DIRECTORY_SERVICE_INTERVAL_DEFAULT
= "24h";
+ @Deprecated
Review Comment:
no longer used.
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/service/DirectoryDeletingService.java:
##########
@@ -76,31 +92,33 @@ public class DirectoryDeletingService extends
AbstractKeyDeletingService {
// Using multi thread for DirDeletion. Multiple threads would read
// from parent directory info from deleted directory table concurrently
// and send deletion requests.
- private final int dirDeletingCorePoolSize;
Review Comment:
Javadoc:
```
/**
* Background service responsible for purging deleted directories and files
* in the Ozone Manager (OM) and associated snapshots.
*
* <p>
* This service periodically scans the deleted directory table and submits
* purge requests for directories and their sub-entries (subdirectories and
files).
* It operates in both the active object store (AOS) and across all
deep-clean enabled
* snapshots. The service supports parallel processing using a thread pool
and
* coordinates exclusive size calculations and cleanup status updates for
* snapshots.
* </p>
*
* <h2>Key Features</h2>
* <ul>
* <li>Processes deleted directories in both the active OM and all
snapshots
* with deep cleaning enabled.</li>
* <li>Uses a thread pool to parallelize deletion tasks within each store
or snapshot.</li>
* <li>Employs filters to determine reclaimability of directories and
files,
* ensuring safety with respect to snapshot chains.</li>
* <li>Tracks and updates exclusive size and replicated exclusive size for
each
* snapshot as directories and files are reclaimed.</li>
* <li>Updates the "deep cleaned" flag for snapshots after a successful
run.</li>
* <li>Handles error and race conditions gracefully, deferring work if
necessary.</li>
* </ul>
*
* <h2>Constructor Parameters</h2>
* <ul>
* <li><b>interval</b> - How often the service runs.</li>
* <li><b>unit</b> - Time unit for the interval.</li>
* <li><b>serviceTimeout</b> - Service timeout in the given time unit.</li>
* <li><b>ozoneManager</b> - The OzoneManager instance.</li>
* <li><b>configuration</b> - Ozone configuration object.</li>
* <li><b>dirDeletingServiceCorePoolSize</b> - Number of parallel threads
for deletion per store or snapshot.</li>
* <li><b>deepCleanSnapshots</b> - Whether to enable deep cleaning for
snapshots.</li>
* </ul>
*
* <h2>Threading and Parallelism</h2>
* <ul>
* <li>Uses a configurable thread pool for parallel deletion tasks within
each store/snapshot.</li>
* <li>Each snapshot and AOS get a separate background task for
deletion.</li>
* </ul>
*
* <h2>Snapshot Integration</h2>
* <ul>
* <li>Iterates all snapshots in the chain if deep cleaning is
enabled.</li>
* <li>Skips snapshots that are already deep-cleaned or not yet flushed to
disk.</li>
* <li>Updates snapshot metadata to reflect size changes and cleaning
status.</li>
* </ul>
*
* <h2>Usage</h2>
* <ul>
* <li>Should be scheduled as a background service in OM.</li>
* <li>Intended to be run only on the OM leader node.</li>
* </ul>
*
* @see org.apache.hadoop.ozone.om.snapshot.filter.ReclaimableDirFilter
* @see org.apache.hadoop.ozone.om.snapshot.filter.ReclaimableKeyFilter
* @see org.apache.hadoop.ozone.om.SnapshotChainManager
*/
```
##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestDirectoryDeletingServiceWithFSO.java:
##########
@@ -749,30 +748,29 @@ public void testDirDeletedTableCleanUpForSnapshot()
throws Exception {
assertSubPathsCount(dirDeletingService::getMovedFilesCount, 0);
assertSubPathsCount(dirDeletingService::getMovedDirsCount, 0);
assertSubPathsCount(dirDeletingService::getDeletedDirsCount, 0);
-
// Case-2) Delete dir
fs.delete(root, true);
// After delete. 5 sub files are still in keyTable.
// 4 dirs in dirTable.
assertTableRowCount(keyTable, 5);
- assertTableRowCount(dirTable, 4);
+ assertTableRowCount(dirTable, 0);
// KeyDeletingService and DirectoryDeletingService will not
// clean up because the paths are part of a snapshot.
// As a result on 1 deleted dir and 3 deleted files will
Review Comment:
```suggestion
// As a result on 5 deleted dir and 3 deleted files will
```
##########
hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/OMConfigKeys.java:
##########
@@ -387,12 +387,16 @@ public final class OMConfigKeys {
*/
public static final String OZONE_SNAPSHOT_DEEP_CLEANING_ENABLED =
"ozone.snapshot.deep.cleaning.enabled";
public static final boolean OZONE_SNAPSHOT_DEEP_CLEANING_ENABLED_DEFAULT =
false;
+ @Deprecated
Review Comment:
can you add a javadoc here saying the following few static variables are
deprecated because snapshot directory service is replaced by directory deleting
service.
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]