[GitHub] [hbase] saintstack commented on a change in pull request #3230: HBASE-25847 More DEBUG and TRACE level logging in CatalogJanitor and HbckChore

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

saintstack commented on a change in pull request #3230:
URL: https://github.com/apache/hbase/pull/3230#discussion_r626778648



##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/master/HbckChore.java
##########
@@ -223,15 +223,37 @@ private void loadRegionsFromInMemoryState() {
           .isTableState(regionInfo.getTable(), TableState.State.DISABLED)) {
         disabledTableRegions.add(regionInfo.getRegionNameAsString());
       }
-      if (regionInfo.isSplitParent()) {
+      if (regionState.isSplit()) {

Review comment:
       +1 on this change.
   
   Was always odd have a Regions' split state recorded in its RegionInfo; RegionInfo does not keep internally if it is being merged or if open, etc. Regions are manipulated by other entities so state belongs elsewhere. +1 on move to regionstate checking instead.

##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/master/janitor/CatalogJanitor.java
##########
@@ -165,13 +168,19 @@ public int scan() throws IOException {
       this.lastReport = scanForReport();
       if (!this.lastReport.isEmpty()) {
         LOG.warn(this.lastReport.toString());
+      } else {
+        LOG.debug(this.lastReport.toString());

Review comment:
       +1

##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/master/janitor/CatalogJanitor.java
##########
@@ -165,13 +168,19 @@ public int scan() throws IOException {
       this.lastReport = scanForReport();
       if (!this.lastReport.isEmpty()) {
         LOG.warn(this.lastReport.toString());
+      } else {
+        LOG.debug(this.lastReport.toString());
       }
+
       updateAssignmentManagerMetrics();
 
       Map<RegionInfo, Result> mergedRegions = this.lastReport.mergedRegions;
       for (Map.Entry<RegionInfo, Result> e : mergedRegions.entrySet()) {
         if (this.services.isInMaintenanceMode()) {
           // Stop cleaning if the master is in maintenance mode
+          if (LOG.isDebugEnabled()) {
+            LOG.debug("In maintenence mode, not cleaning");
+          }

Review comment:
       We need the if (LOG.isDebug.... ?




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