[GitHub] [hbase] liuml07 commented on a change in pull request #2076: HBASE-24740 Enable journal logging for HBase snapshot operation

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

liuml07 commented on a change in pull request #2076:
URL: https://github.com/apache/hbase/pull/2076#discussion_r456248801



##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/snapshot/SnapshotManifest.java
##########
@@ -343,6 +343,13 @@ private void load() throws IOException {
     }
   }
 
+  /**
+   * Sets the status task for monitoring all the subtasks for Snapshot operation
+   */
+  public void setMonitoredTask(MonitoredTask statusTask) {

Review comment:
       It seems more straight to readers if we pass this in the `create()`. Also this class field can be final then. @sguggilam We can have two versions of `create()` I guess? The one supporting those call sites which do not need monitored task does not need to change.

##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/monitoring/MonitoredTaskImpl.java
##########
@@ -18,15 +18,13 @@
  */
 package org.apache.hadoop.hbase.monitoring;
 
-import org.apache.hadoop.hbase.classification.InterfaceAudience;
-
 import java.io.IOException;
 import java.util.ArrayList;
 import java.util.Collections;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
-
+import org.apache.hadoop.hbase.classification.InterfaceAudience;

Review comment:
       Is this required? 




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