[ https://issues.apache.org/jira/browse/HBASE-16469?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15965076#comment-15965076 ]
Hudson commented on HBASE-16469: -------------------------------- FAILURE: Integrated in Jenkins build HBase-1.4 #692 (See [https://builds.apache.org/job/HBase-1.4/692/]) HBASE-16469 Several log refactoring/improvement suggestions (busbey: rev 4030facc990b95fd6f17499f1597f0892fdcb449) * (edit) hbase-server/src/test/java/org/apache/hadoop/hbase/util/MultiThreadedUpdater.java * (edit) hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java * (edit) hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/handler/CloseRegionHandler.java * (edit) hbase-it/src/test/java/org/apache/hadoop/hbase/HBaseClusterManager.java * (edit) hbase-client/src/main/java/org/apache/hadoop/hbase/client/PreemptiveFastFailInterceptor.java > Several log refactoring/improvement suggestions > ----------------------------------------------- > > Key: HBASE-16469 > URL: https://issues.apache.org/jira/browse/HBASE-16469 > Project: HBase > Issue Type: Improvement > Components: Operability > Affects Versions: 1.2.5 > Reporter: Nemo Chen > Assignee: Nemo Chen > Labels: easyfix, easytest > Fix For: 2.0.0, 1.4.0 > > Attachments: HBASE-16469.master.001.patch > > > *method invocation replaced by variable* > hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/handler/CloseRegionHandler.java > {code}String name = regionInfo.getRegionNameAsString();{code} > {code}LOG.warn("Can't close region: was already closed during close(): " + > regionInfo.getRegionNameAsString()); {code} > In the above two examples, the method invocations are assigned to the > variables before the logging code. These method invocations should be > replaced by variables in case of simplicity and readability > ---- > *method invocation in return statement* > hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java > {code} > public String toString() { > return getRegionInfo().getRegionNameAsString(); > } > {code} > {code} > LOG.debug("Region " + getRegionInfo().getRegionNameAsString() > + " is not mergeable because it is closing or closed"); > {code} > {code} > LOG.debug("Region " + getRegionInfo().getRegionNameAsString() > + " is not mergeable because it has references"); > {code} > {code} > LOG.info("Running close preflush of " + > getRegionInfo().getRegionNameAsString()); > {code} > In these above examples, the "getRegionInfo().getRegionNameAsString())" is > the return statement of method "toString" in the same class. They should be > replaced with “this” in case of simplicity and readability. > ---- > *check the logged variable if it is null* > hbase-it/src/test/java/org/apache/hadoop/hbase/HBaseClusterManager.java > {code} > if ((sshUserName != null && sshUserName.length() > 0) || > (sshOptions != null && sshOptions.length() > 0)) { > LOG.info("Running with SSH user [" + sshUserName + "] and options [" + > sshOptions + "]"); > } > {code} > hbase-server/src/main/java/org/apache/hadoop/hbase/master/AssignmentManager.java > {code} > if ((regionState == null && latestState != null) > || (regionState != null && latestState == null) > || (regionState != null && latestState != null > && latestState.getState() != regionState.getState())) { > LOG.warn("Region state changed from " + regionState + " to " > + latestState + ", while acquiring lock"); > } > {code} > In the above example, the logging variable could null at run time. It is a > bad practice to include null variables inside logs. > ---- > *variable in byte printed directly* > hbase-server/src/test/java/org/apache/hadoop/hbase/util/MultiThreadedUpdater.java > {code} > byte[] rowKey = dataGenerator.getDeterministicUniqueKey(rowKeyBase); > {code} > {code} > LOG.error("Failed to update the row with key = [" + rowKey > + "], since we could not get the original row"); > {code} > rowKey should be printed as Bytes.toString(rowKey). > ---- > *object toString contain mi* > The toString method returns getServerName(), so the "server.getServerName()" > should be replaced with "server" in case of simplicity and readability. > hbase-client/src/main/java/org/apache/hadoop/hbase/client/PreemptiveFastFailInterceptor.java > {code} > LOG.info("Clearing out PFFE for server " + server.getServerName()); > return getServerName(); > {code} -- This message was sent by Atlassian JIRA (v6.3.15#6346)