You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by "xichen01 (via GitHub)" <gi...@apache.org> on 2025/12/04 06:49:19 UTC

[PR] HDDS-12791. Make Retention Service suspendable [ozone]

xichen01 opened a new pull request, #9426:
URL: https://github.com/apache/ozone/pull/9426

   ## What changes were proposed in this pull request?
   This PR makes the KeyLifecycleService  suspendable at runtime without requiring an OM restart.
   
   - Make `ozone.lifecycle.service.enabled` reconfigurable, the KeyLifecycleService can be stopped by reconfiguring this configuration.
   - Add KeyLifecycleService status query command
   Example:
   ```bash
   $ ozone admin om lifecycle status --service-id <omServiceId>
   ========================================
             Lifecycle Service Status
   ========================================
   IsEnabled: true
   IsRunning: true
   Running Buckets:
     - /vol1/bucket1
   ========================================
   ```
   
   
   ## What is the link to the Apache JIRA
   https://issues.apache.org/jira/browse/HDDS-12791
   
   ## How was this patch tested?
   unit test.
   


-- 
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: issues-unsubscribe@ozone.apache.org

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


Re: [PR] HDDS-12791. Make Retention Service suspendable [ozone]

Posted by "ChenSammi (via GitHub)" <gi...@apache.org>.
ChenSammi commented on code in PR #9426:
URL: https://github.com/apache/ozone/pull/9426#discussion_r2605454016


##########
hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/admin/om/LifecycleStatusSubCommand.java:
##########
@@ -0,0 +1,88 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.ozone.admin.om;
+
+import java.io.IOException;
+import java.io.PrintStream;
+import java.util.concurrent.Callable;
+import org.apache.hadoop.hdds.cli.HddsVersionProvider;
+import org.apache.hadoop.ozone.om.protocol.OzoneManagerProtocol;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.GetLifecycleServiceStatusResponse;
+import picocli.CommandLine;
+import picocli.CommandLine.Command;
+
+/**
+ * Handler of ozone admin status command.
+ */
+@Command(
+    name = "status",

Review Comment:
   We have new CLI to query the service status, and leverage the reconfiguration framework for the service start/stop, can we use consolidate to one entry, for example, new CLI for both start/stop/status?  Current both status CLI and reconfiguration with both ozone-site.xml content change and reconfiguration CLI, feels a little complicated. 



##########
hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/admin/om/LifecycleStatusSubCommand.java:
##########
@@ -0,0 +1,88 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.ozone.admin.om;
+
+import java.io.IOException;
+import java.io.PrintStream;
+import java.util.concurrent.Callable;
+import org.apache.hadoop.hdds.cli.HddsVersionProvider;
+import org.apache.hadoop.ozone.om.protocol.OzoneManagerProtocol;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.GetLifecycleServiceStatusResponse;
+import picocli.CommandLine;
+import picocli.CommandLine.Command;
+
+/**
+ * Handler of ozone admin status command.
+ */
+@Command(
+    name = "status",

Review Comment:
   We have new CLI to query the service status, and leverage the reconfiguration framework for the service start/stop, can we use consolidate to one entry, for example, new CLI for both start/stop/status?  Current both status CLI and reconfiguration with both ozone-site.xml content change and reconfiguration CLI, feels a little complicated for user.



-- 
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: issues-unsubscribe@ozone.apache.org

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


Re: [PR] HDDS-12791. Make Retention Service suspendable [ozone]

Posted by "ChenSammi (via GitHub)" <gi...@apache.org>.
ChenSammi commented on code in PR #9426:
URL: https://github.com/apache/ozone/pull/9426#discussion_r2605391331


##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java:
##########
@@ -5157,6 +5176,20 @@ private String reconfOzoneKeyDeletingLimitPerTask(String newVal) {
     return newVal;
   }
 
+  private String reconfKeyLifecycleServiceEnabled(String newVal) {
+    boolean enabled = StringUtils.isEmpty(newVal) ?
+        OZONE_KEY_LIFECYCLE_SERVICE_ENABLED_DEFAULT : Boolean.parseBoolean(newVal);
+    KeyLifecycleService keyLifecycleService = getKeyManager().getKeyLifecycleService();
+    if (keyLifecycleService != null) {
+      keyLifecycleService.setServiceEnabled(enabled);
+    } else {
+      LOG.warn("Failed reconfiguration for '{}'. KeyLifecycleService is not initialized.",
+          OZONE_KEY_LIFECYCLE_SERVICE_ENABLED);
+    }
+    getConfiguration().setBoolean(OZONE_KEY_LIFECYCLE_SERVICE_ENABLED, enabled);

Review Comment:
   Let's change the configuration first, and then update the service state. 



-- 
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: issues-unsubscribe@ozone.apache.org

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


Re: [PR] HDDS-12791. Make Retention Service suspendable [ozone]

Posted by "ChenSammi (via GitHub)" <gi...@apache.org>.
ChenSammi commented on code in PR #9426:
URL: https://github.com/apache/ozone/pull/9426#discussion_r2605457897


##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/service/KeyLifecycleService.java:
##########
@@ -698,6 +724,9 @@ private void evaluateBucket(OmBucketInfo bucketInfo,
       try (TableIterator<String, ? extends Table.KeyValue<String, OmKeyInfo>> keyTblItr =
                keyTable.iterator(omMetadataManager.getBucketKey(volumeName, bucketName))) {
         while (keyTblItr.hasNext()) {
+          if (!shouldRun()) {
+            return;

Review Comment:
   Can we add a log message here too about the abort of bucket key evaluation? 



-- 
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: issues-unsubscribe@ozone.apache.org

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


Re: [PR] HDDS-12791. Make Retention Service suspendable [ozone]

Posted by "ChenSammi (via GitHub)" <gi...@apache.org>.
ChenSammi merged PR #9426:
URL: https://github.com/apache/ozone/pull/9426


-- 
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: issues-unsubscribe@ozone.apache.org

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


Re: [PR] HDDS-12791. Make Retention Service suspendable [ozone]

Posted by "ChenSammi (via GitHub)" <gi...@apache.org>.
ChenSammi commented on code in PR #9426:
URL: https://github.com/apache/ozone/pull/9426#discussion_r2605390118


##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java:
##########
@@ -5157,6 +5176,20 @@ private String reconfOzoneKeyDeletingLimitPerTask(String newVal) {
     return newVal;
   }
 
+  private String reconfKeyLifecycleServiceEnabled(String newVal) {
+    boolean enabled = StringUtils.isEmpty(newVal) ?

Review Comment:
   Can we throw exception if newVal is null?  I think "null" is an invalid input, so it's better response to user with correct info. Will the message in exception be passed to client and show as the output of CLI? 



-- 
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: issues-unsubscribe@ozone.apache.org

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


Re: [PR] HDDS-12791. Make Retention Service suspendable [ozone]

Posted by "ChenSammi (via GitHub)" <gi...@apache.org>.
ChenSammi commented on code in PR #9426:
URL: https://github.com/apache/ozone/pull/9426#discussion_r2629754939


##########
hadoop-ozone/interface-client/src/main/proto/OmClientProtocol.proto:
##########
@@ -2432,3 +2440,18 @@ service OzoneManagerService {
     rpc submitRequest(OMRequest)
           returns(OMResponse);
 }
+
+message GetLifecycleServiceStatusRequest {
+}
+
+message GetLifecycleServiceStatusResponse {
+  required bool isEnabled = 1;

Review Comment:
   Can we add one more state, to indicate that service is suspened/paused or not? 



-- 
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: issues-unsubscribe@ozone.apache.org

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


Re: [PR] HDDS-12791. Make Retention Service suspendable [ozone]

Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #9426:
URL: https://github.com/apache/ozone/pull/9426#discussion_r2597026647


##########
hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/service/TestKeyLifecycleService.java:
##########
@@ -1537,6 +1539,48 @@ void testMultipleDirectoriesMatched(String keyPrefix1, String keyPrefix2, String
       }
       deleteLifecyclePolicy(volumeName, bucketName);
     }
+
+    @Test
+    void testGetLifecycleServiceStatus() throws Exception {
+      final String volumeName = getTestName();
+      final String bucketName = uniqueObjectName("bucket");
+      String prefix = "key";
+      
+      //Sservice should be enabled but not running

Review Comment:
   Spelling error in comment: "Sservice" should be "Service".
   ```suggestion
         //Service should be enabled but not running
   ```



##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/service/KeyLifecycleService.java:
##########
@@ -219,12 +221,26 @@ public void resume() {
     suspended.set(false);
   }
 
+  public void setServiceEnabled(boolean enabled) {
+    this.isServiceEnabled.set(enabled);
+    LOG.info("KeyLifecycleService is {}", enabled ? "enabled" : "disabled");
+  }

Review Comment:
   Missing JavaDoc for public method `setServiceEnabled`. This method enables or disables the lifecycle service at runtime and is part of the reconfiguration feature, so it should be documented to explain its purpose and parameters.



##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/service/KeyLifecycleService.java:
##########
@@ -219,12 +221,26 @@ public void resume() {
     suspended.set(false);
   }
 
+  public void setServiceEnabled(boolean enabled) {
+    this.isServiceEnabled.set(enabled);
+    LOG.info("KeyLifecycleService is {}", enabled ? "enabled" : "disabled");
+  }
+
   @Override
   public void shutdown() {
     super.shutdown();
     KeyLifecycleServiceMetrics.unregister();
   }
 
+  public GetLifecycleServiceStatusResponse status() {
+    Set<String> runningBuckets = new HashSet<>(inFlight.keySet());
+    return GetLifecycleServiceStatusResponse.newBuilder()
+        .setIsRunning(!runningBuckets.isEmpty())
+        .setIsEnabled(isServiceEnabled.get())
+        .addAllRunningBuckets(runningBuckets)
+        .build();
+  }

Review Comment:
   Missing JavaDoc for public method `status`. This method returns the current status of the lifecycle service (enabled/running state and buckets being processed) and should be documented to explain what it returns and when it should be called.



##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/service/KeyLifecycleService.java:
##########
@@ -470,6 +491,11 @@ private void evaluateKeyAndDirTable(OmBucketInfo bucket, long volumeObjId, Table
 
       HashSet<Long> deletedDirSet = new HashSet<>();
       while (!stack.isEmpty()) {
+        if (!shouldRun()) {
+          LOG.info("KeyLifecycleService is suspended or disabled." +

Review Comment:
   This string appears to be missing a space after 'disabled.'.
   ```suggestion
             LOG.info("KeyLifecycleService is suspended or disabled. " +
   ```



-- 
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: issues-unsubscribe@ozone.apache.org

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


Re: [PR] HDDS-12791. Make Retention Service suspendable [ozone]

Posted by "xichen01 (via GitHub)" <gi...@apache.org>.
xichen01 commented on code in PR #9426:
URL: https://github.com/apache/ozone/pull/9426#discussion_r2622650131


##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/service/KeyLifecycleService.java:
##########
@@ -698,6 +724,9 @@ private void evaluateBucket(OmBucketInfo bucketInfo,
       try (TableIterator<String, ? extends Table.KeyValue<String, OmKeyInfo>> keyTblItr =
                keyTable.iterator(omMetadataManager.getBucketKey(volumeName, bucketName))) {
         while (keyTblItr.hasNext()) {
+          if (!shouldRun()) {
+            return;

Review Comment:
   Added.



##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/service/KeyLifecycleService.java:
##########
@@ -331,6 +347,11 @@ public BackgroundTaskResult call() {
           return result;
         }
 
+        if (!shouldRun()) {

Review Comment:
   removed



-- 
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: issues-unsubscribe@ozone.apache.org

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


Re: [PR] HDDS-12791. Make Retention Service suspendable [ozone]

Posted by "ChenSammi (via GitHub)" <gi...@apache.org>.
ChenSammi commented on code in PR #9426:
URL: https://github.com/apache/ozone/pull/9426#discussion_r2629751342


##########
hadoop-ozone/interface-client/src/main/proto/OmClientProtocol.proto:
##########
@@ -2432,3 +2440,18 @@ service OzoneManagerService {
     rpc submitRequest(OMRequest)
           returns(OMResponse);
 }
+
+message GetLifecycleServiceStatusRequest {
+}
+
+message GetLifecycleServiceStatusResponse {
+  required bool isEnabled = 1;
+  repeated string runningBuckets = 2;
+}
+
+message SuspendLifecycleServiceRequest {

Review Comment:
   Can we add an option in the SuspendLifecycleServiceRequest so that we can resume the service once suspend is no longer needed? 



-- 
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: issues-unsubscribe@ozone.apache.org

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


Re: [PR] HDDS-12791. Make Retention Service suspendable [ozone]

Posted by "ChenSammi (via GitHub)" <gi...@apache.org>.
ChenSammi commented on PR #9426:
URL: https://github.com/apache/ozone/pull/9426#issuecomment-3696134401

   Thanks @xichen01 for updating the patch.  The last commit looks overall good to me, +1. 


-- 
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: issues-unsubscribe@ozone.apache.org

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


Re: [PR] HDDS-12791. Make Retention Service suspendable [ozone]

Posted by "xichen01 (via GitHub)" <gi...@apache.org>.
xichen01 commented on code in PR #9426:
URL: https://github.com/apache/ozone/pull/9426#discussion_r2622651425


##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/service/KeyLifecycleService.java:
##########
@@ -219,12 +221,26 @@ public void resume() {
     suspended.set(false);
   }
 
+  public void setServiceEnabled(boolean enabled) {
+    this.isServiceEnabled.set(enabled);
+    LOG.info("KeyLifecycleService is {}", enabled ? "enabled" : "disabled");
+  }
+
   @Override
   public void shutdown() {
     super.shutdown();
     KeyLifecycleServiceMetrics.unregister();
   }
 
+  public GetLifecycleServiceStatusResponse status() {
+    Set<String> runningBuckets = new HashSet<>(inFlight.keySet());
+    return GetLifecycleServiceStatusResponse.newBuilder()
+        .setIsRunning(!runningBuckets.isEmpty())
+        .setIsEnabled(isServiceEnabled.get())

Review Comment:
   Removed the setIsRunning.



-- 
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: issues-unsubscribe@ozone.apache.org

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


Re: [PR] HDDS-12791. Make Retention Service suspendable [ozone]

Posted by "ChenSammi (via GitHub)" <gi...@apache.org>.
ChenSammi commented on code in PR #9426:
URL: https://github.com/apache/ozone/pull/9426#discussion_r2605454016


##########
hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/admin/om/LifecycleStatusSubCommand.java:
##########
@@ -0,0 +1,88 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.ozone.admin.om;
+
+import java.io.IOException;
+import java.io.PrintStream;
+import java.util.concurrent.Callable;
+import org.apache.hadoop.hdds.cli.HddsVersionProvider;
+import org.apache.hadoop.ozone.om.protocol.OzoneManagerProtocol;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.GetLifecycleServiceStatusResponse;
+import picocli.CommandLine;
+import picocli.CommandLine.Command;
+
+/**
+ * Handler of ozone admin status command.
+ */
+@Command(
+    name = "status",

Review Comment:
   We have new CLI to query the service status, and leverage the reconfiguration framework for the service start/stop, can we use consolidate to one entry, for example, new CLI for both start/stop/status?  Current both CLI and reconfiguration ozone-site.xml content change with CLI, feels a little complicated. 



-- 
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: issues-unsubscribe@ozone.apache.org

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


Re: [PR] HDDS-12791. Make Retention Service suspendable [ozone]

Posted by "xichen01 (via GitHub)" <gi...@apache.org>.
xichen01 commented on code in PR #9426:
URL: https://github.com/apache/ozone/pull/9426#discussion_r2642578313


##########
hadoop-ozone/interface-client/src/main/proto/OmClientProtocol.proto:
##########
@@ -2432,3 +2440,18 @@ service OzoneManagerService {
     rpc submitRequest(OMRequest)
           returns(OMResponse);
 }
+
+message GetLifecycleServiceStatusRequest {
+}
+
+message GetLifecycleServiceStatusResponse {
+  required bool isEnabled = 1;

Review Comment:
   Added the suspened flag



##########
hadoop-ozone/interface-client/src/main/proto/OmClientProtocol.proto:
##########
@@ -2432,3 +2440,18 @@ service OzoneManagerService {
     rpc submitRequest(OMRequest)
           returns(OMResponse);
 }
+
+message GetLifecycleServiceStatusRequest {
+}
+
+message GetLifecycleServiceStatusResponse {
+  required bool isEnabled = 1;
+  repeated string runningBuckets = 2;
+}
+
+message SuspendLifecycleServiceRequest {

Review Comment:
   Add a resume command. new we can suspend and resume the lifecycle service



##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/service/KeyLifecycleService.java:
##########
@@ -219,12 +221,33 @@ public void resume() {
     suspended.set(false);
   }
 
+  /**
+   * Set isServiceEnabled.
+   * @param enabled whether enable the lifecycle Service
+   */
+  public void setServiceEnabled(boolean enabled) {

Review Comment:
   I reused the `suspended` variable  and the the `isServiceEnabled` only based on the Configuration, cannot modify by user



-- 
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: issues-unsubscribe@ozone.apache.org

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


Re: [PR] HDDS-12791. Make Retention Service suspendable [ozone]

Posted by "ChenSammi (via GitHub)" <gi...@apache.org>.
ChenSammi commented on code in PR #9426:
URL: https://github.com/apache/ozone/pull/9426#discussion_r2605462760


##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/service/KeyLifecycleService.java:
##########
@@ -331,6 +347,11 @@ public BackgroundTaskResult call() {
           return result;
         }
 
+        if (!shouldRun()) {

Review Comment:
   When it comes here, the key evaluation is finished, the rest of work is sending the expired remaining key to delete/rename. I think we can let this run to finish, instead of return here.  
   
   The most time consuming part of DB key evaluation iteration, send keys to delete/rename doesn't take much time. 



-- 
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: issues-unsubscribe@ozone.apache.org

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


Re: [PR] HDDS-12791. Make Retention Service suspendable [ozone]

Posted by "ChenSammi (via GitHub)" <gi...@apache.org>.
ChenSammi commented on code in PR #9426:
URL: https://github.com/apache/ozone/pull/9426#discussion_r2605409442


##########
hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/admin/om/FinalizationStatusSubCommand.java:
##########
@@ -52,12 +52,12 @@ public class FinalizationStatusSubCommand implements Callable<Void> {
 
   @Override
   public Void call() throws Exception {
-    OzoneManagerProtocol client =
-        parent.createOmClient(omServiceId, omHost, false);
-    String upgradeClientID = "Upgrade-Client-" + UUID.randomUUID().toString();
-    UpgradeFinalization.StatusAndMessages progress =
-        client.queryUpgradeFinalizationProgress(upgradeClientID, false, true);
-    System.out.println(progress.status());
+    try (OzoneManagerProtocol client = parent.createOmClient(omServiceId, omHost, false)) {

Review Comment:
   Is this a relevant change ? 



-- 
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: issues-unsubscribe@ozone.apache.org

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


Re: [PR] HDDS-12791. Make Retention Service suspendable [ozone]

Posted by "ChenSammi (via GitHub)" <gi...@apache.org>.
ChenSammi commented on code in PR #9426:
URL: https://github.com/apache/ozone/pull/9426#discussion_r2605469288


##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/service/KeyLifecycleService.java:
##########
@@ -219,12 +221,26 @@ public void resume() {
     suspended.set(false);
   }
 
+  public void setServiceEnabled(boolean enabled) {
+    this.isServiceEnabled.set(enabled);
+    LOG.info("KeyLifecycleService is {}", enabled ? "enabled" : "disabled");
+  }
+
   @Override
   public void shutdown() {
     super.shutdown();
     KeyLifecycleServiceMetrics.unregister();
   }
 
+  public GetLifecycleServiceStatusResponse status() {
+    Set<String> runningBuckets = new HashSet<>(inFlight.keySet());
+    return GetLifecycleServiceStatusResponse.newBuilder()
+        .setIsRunning(!runningBuckets.isEmpty())
+        .setIsEnabled(isServiceEnabled.get())

Review Comment:
   Can we combine the IsRunning and IsEnabled to one state.  If IsRuning is the result of whether there is bucket task running or not, this info is already covered by RunningBuckets fields. 



-- 
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: issues-unsubscribe@ozone.apache.org

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


Re: [PR] HDDS-12791. Make Retention Service suspendable [ozone]

Posted by "xichen01 (via GitHub)" <gi...@apache.org>.
xichen01 commented on code in PR #9426:
URL: https://github.com/apache/ozone/pull/9426#discussion_r2622649495


##########
hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/admin/om/LifecycleStatusSubCommand.java:
##########
@@ -0,0 +1,88 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.ozone.admin.om;
+
+import java.io.IOException;
+import java.io.PrintStream;
+import java.util.concurrent.Callable;
+import org.apache.hadoop.hdds.cli.HddsVersionProvider;
+import org.apache.hadoop.ozone.om.protocol.OzoneManagerProtocol;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.GetLifecycleServiceStatusResponse;
+import picocli.CommandLine;
+import picocli.CommandLine.Command;
+
+/**
+ * Handler of ozone admin status command.
+ */
+@Command(
+    name = "status",

Review Comment:
   Added suspend subcommand.



-- 
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: issues-unsubscribe@ozone.apache.org

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


Re: [PR] HDDS-12791. Make Retention Service suspendable [ozone]

Posted by "xichen01 (via GitHub)" <gi...@apache.org>.
xichen01 commented on code in PR #9426:
URL: https://github.com/apache/ozone/pull/9426#discussion_r2622648204


##########
hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/admin/om/FinalizationStatusSubCommand.java:
##########
@@ -52,12 +52,12 @@ public class FinalizationStatusSubCommand implements Callable<Void> {
 
   @Override
   public Void call() throws Exception {
-    OzoneManagerProtocol client =
-        parent.createOmClient(omServiceId, omHost, false);
-    String upgradeClientID = "Upgrade-Client-" + UUID.randomUUID().toString();
-    UpgradeFinalization.StatusAndMessages progress =
-        client.queryUpgradeFinalizationProgress(upgradeClientID, false, true);
-    System.out.println(progress.status());
+    try (OzoneManagerProtocol client = parent.createOmClient(omServiceId, omHost, false)) {

Review Comment:
   removed.



-- 
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: issues-unsubscribe@ozone.apache.org

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


Re: [PR] HDDS-12791. Make Retention Service suspendable [ozone]

Posted by "ChenSammi (via GitHub)" <gi...@apache.org>.
ChenSammi commented on code in PR #9426:
URL: https://github.com/apache/ozone/pull/9426#discussion_r2629744814


##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/service/KeyLifecycleService.java:
##########
@@ -219,12 +221,33 @@ public void resume() {
     suspended.set(false);
   }
 
+  /**
+   * Set isServiceEnabled.
+   * @param enabled whether enable the lifecycle Service
+   */
+  public void setServiceEnabled(boolean enabled) {

Review Comment:
   Can we reuse the existing suspended variable, and suspend(), resume() function, instead of this new setServiceEnabled() function, as the current suspended variable and it involved function is actually not used? 



-- 
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: issues-unsubscribe@ozone.apache.org

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