Copilot commented on code in PR #9155:
URL: https://github.com/apache/ozone/pull/9155#discussion_r2431414815


##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java:
##########
@@ -3554,6 +3554,53 @@ public boolean triggerRangerBGSync(boolean noWait) 
throws IOException {
     }
   }
 
+  @Override
+  public boolean triggerSnapshotDefrag(boolean noWait) throws IOException {
+
+    // OM should be leader and ready.
+    if (!isLeaderReady()) {
+      throw new OMException("OM is not leader or not ready", INVALID_REQUEST);
+    }
+
+    final UserGroupInformation ugi = getRemoteUser();
+    // Check Ozone admin privilege
+    if (!isAdmin(ugi)) {
+      throw new OMException("Only Ozone admins are allowed to trigger "
+          + "snapshot defragmentation manually", PERMISSION_DENIED);
+    }
+
+    // Get the SnapshotDefragService from KeyManager
+    final SnapshotDefragService defragService = 
keyManager.getSnapshotDefragService();
+    if (defragService == null) {
+      throw new OMException("Snapshot defragmentation service is not 
initialized",
+          FEATURE_NOT_ENABLED);
+    }
+
+    // Trigger Snapshot Defragmentation
+    if (noWait) {
+      final Thread t = new Thread(() -> {
+        try {
+          defragService.start();

Review Comment:
   [nitpick] The method name 'start()' is ambiguous for a defragmentation 
service. Consider using a more descriptive method name like 
'triggerDefragmentation()' or 'runDefragmentation()'.



##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java:
##########
@@ -3554,6 +3554,53 @@ public boolean triggerRangerBGSync(boolean noWait) 
throws IOException {
     }
   }
 
+  @Override
+  public boolean triggerSnapshotDefrag(boolean noWait) throws IOException {
+
+    // OM should be leader and ready.
+    if (!isLeaderReady()) {
+      throw new OMException("OM is not leader or not ready", INVALID_REQUEST);
+    }
+
+    final UserGroupInformation ugi = getRemoteUser();
+    // Check Ozone admin privilege
+    if (!isAdmin(ugi)) {
+      throw new OMException("Only Ozone admins are allowed to trigger "
+          + "snapshot defragmentation manually", PERMISSION_DENIED);
+    }
+
+    // Get the SnapshotDefragService from KeyManager
+    final SnapshotDefragService defragService = 
keyManager.getSnapshotDefragService();
+    if (defragService == null) {
+      throw new OMException("Snapshot defragmentation service is not 
initialized",
+          FEATURE_NOT_ENABLED);
+    }
+
+    // Trigger Snapshot Defragmentation
+    if (noWait) {
+      final Thread t = new Thread(() -> {
+        try {
+          defragService.start();
+        } catch (Exception e) {
+          LOG.error("Error during snapshot defragmentation", e);
+        }
+      }, threadPrefix + "SnapshotDefrag");

Review Comment:
   [nitpick] The thread name 'SnapshotDefrag' should be more descriptive to 
include a timestamp or unique identifier for better debugging and monitoring.
   ```suggestion
         }, threadPrefix + "SnapshotDefrag-" + System.currentTimeMillis());
   ```



##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java:
##########
@@ -3554,6 +3554,53 @@ public boolean triggerRangerBGSync(boolean noWait) 
throws IOException {
     }
   }
 
+  @Override
+  public boolean triggerSnapshotDefrag(boolean noWait) throws IOException {
+
+    // OM should be leader and ready.
+    if (!isLeaderReady()) {
+      throw new OMException("OM is not leader or not ready", INVALID_REQUEST);
+    }
+
+    final UserGroupInformation ugi = getRemoteUser();
+    // Check Ozone admin privilege
+    if (!isAdmin(ugi)) {
+      throw new OMException("Only Ozone admins are allowed to trigger "
+          + "snapshot defragmentation manually", PERMISSION_DENIED);
+    }
+
+    // Get the SnapshotDefragService from KeyManager
+    final SnapshotDefragService defragService = 
keyManager.getSnapshotDefragService();
+    if (defragService == null) {
+      throw new OMException("Snapshot defragmentation service is not 
initialized",
+          FEATURE_NOT_ENABLED);
+    }
+
+    // Trigger Snapshot Defragmentation
+    if (noWait) {
+      final Thread t = new Thread(() -> {
+        try {
+          defragService.start();
+        } catch (Exception e) {
+          LOG.error("Error during snapshot defragmentation", e);
+        }
+      }, threadPrefix + "SnapshotDefrag");
+      t.start();
+      LOG.info("User '{}' manually triggered Snapshot Defragmentation without 
waiting"
+          + " in a new thread, tid = {}", ugi, t.getId());
+      return true;
+    } else {
+      LOG.info("User '{}' manually triggered Snapshot Defragmentation and is 
waiting", ugi);
+      try {
+        defragService.start();

Review Comment:
   [nitpick] The method name 'start()' is ambiguous for a defragmentation 
service. Consider using a more descriptive method name like 
'triggerDefragmentation()' or 'runDefragmentation()'.



-- 
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