[GitHub] [accumulo] ctubbsii commented on a change in pull request #1339: Reduce merge operations logging of extent info at - issue #1338

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

ctubbsii commented on a change in pull request #1339: Reduce merge operations logging of extent info at - issue #1338
URL: https://github.com/apache/accumulo/pull/1339#discussion_r320979507
 
 

 ##########
 File path: server/master/src/main/java/org/apache/accumulo/master/Master.java
 ##########
 @@ -872,8 +872,16 @@ TabletGoalState getGoalState(TabletLocationState tls, MergeInfo mergeInfo) {
       }
       // Handle merge transitions
       if (mergeInfo.getExtent() != null) {
-        log.debug("mergeInfo overlaps: " + extent + " " + mergeInfo.overlaps(extent));
-        if (mergeInfo.overlaps(extent)) {
+
+        final boolean overlaps = mergeInfo.overlaps(extent);
+
+        if (overlaps && log.isDebugEnabled()) {
+          log.debug("mergeInfo overlaps: {} {}", extent, overlaps);
+        } else if (log.isTraceEnabled()) {
+          log.trace("mergeInfo overlaps: {} {}", extent, overlaps);
 
 Review comment:
   I think it'd be best to change this implementation by removing:
   
   ```java
     if (log.isTraceEnabled()) {
       log.trace("mergeInfo overlaps: {} {}", extent, overlaps);
     } else if (overlaps) {
       log.debug("mergeInfo overlaps: {} true", extent);
     }
   ```
   
   and putting the debug log message into the if statement below, to be:
   
   ```java
     if (overlaps) {
       log.debug("mergeInfo overlaps {}", extent);
       // ...
     } else {
       log.trace("mergeInfo does not overlap {}", extent);
     }
   ```

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


With regards,
Apache Git Services