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]

Reply via email to