[GitHub] [ozone] GlenGeng commented on a change in pull request #1936: HDDS-4807. Add install checkpoint in SCMStateMachine.

[GitBox <gi...@apache.org>]

GlenGeng commented on a change in pull request #1936:
URL: https://github.com/apache/ozone/pull/1936#discussion_r585230910



##########
File path: hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/SCMBlockDeletingService.java
##########
@@ -248,4 +248,9 @@ public boolean shouldRun() {
   public String getServiceName() {
     return SCMBlockDeletingService.class.getSimpleName();
   }
+
+  @Override
+  public void stop() {

Review comment:
       > What do you think?
   I agree.
   
   > However, a start and stop method in such services are desired so that, all the registered services can be started and stopped via service manager itself.
   In order to support safely stop() then start(), these service should be rewritten to avoid use ExecutorService. This can be done as a newbie task in future.
   




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

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