You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@hbase.apache.org by GitBox <gi...@apache.org> on 2020/07/20 23:30:21 UTC

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

bharathv commented on a change in pull request #2104:
URL: https://github.com/apache/hbase/pull/2104#discussion_r457746383



##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/snapshot/SnapshotManifest.java
##########
@@ -478,7 +494,7 @@ public void consolidate() throws IOException {
       FSTableDescriptors.createTableDescriptorForTableDirectory(workingDirFs, workingDir, htd,
           false);
     } else {
-      LOG.debug("Convert to Single Snapshot Manifest");
+      LOG.debug("Convert to Single Snapshot Manifest for " + this.desc.getName());

Review comment:
       nit: parameterized logging

##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/master/snapshot/SnapshotManager.java
##########
@@ -654,14 +654,16 @@ private void takeSnapshotInternal(SnapshotDescription snapshot) throws IOExcepti
     TableName snapshotTable = TableName.valueOf(snapshot.getTable());
     if (master.getTableStateManager().isTableState(snapshotTable,
         TableState.State.ENABLED)) {
-      LOG.debug("Table enabled, starting distributed snapshot.");
+      LOG.debug("Table enabled, starting distributed snapshots for {}",

Review comment:
       nit: This still requires the isDebugEnabled() guard because ClientSnapshotDescriptionUtils.toString(..) is not a trivial call.

##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/master/snapshot/SnapshotManager.java
##########
@@ -654,14 +654,16 @@ private void takeSnapshotInternal(SnapshotDescription snapshot) throws IOExcepti
     TableName snapshotTable = TableName.valueOf(snapshot.getTable());
     if (master.getTableStateManager().isTableState(snapshotTable,
         TableState.State.ENABLED)) {
-      LOG.debug("Table enabled, starting distributed snapshot.");
+      LOG.debug("Table enabled, starting distributed snapshots for {}",
+        ClientSnapshotDescriptionUtils.toString(snapshot));
       snapshotEnabledTable(snapshot);
       LOG.debug("Started snapshot: " + ClientSnapshotDescriptionUtils.toString(snapshot));
     }
     // For disabled table, snapshot is created by the master
     else if (master.getTableStateManager().isTableState(snapshotTable,
         TableState.State.DISABLED)) {
-      LOG.debug("Table is disabled, running snapshot entirely on master.");
+        LOG.debug("Table is disabled, running snapshot entirely on master for {}",

Review comment:
       same as above.




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