[ https://issues.apache.org/jira/browse/HBASE-16469?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Nemo Chen updated HBASE-16469: ------------------------------ Attachment: HBASE-16469.git3f671c1ead.001.patch > Several log refactoring/improvement suggestions > ----------------------------------------------- > > Key: HBASE-16469 > URL: https://issues.apache.org/jira/browse/HBASE-16469 > Project: HBase > Issue Type: Bug > Affects Versions: 1.2.2 > Reporter: Nemo Chen > Labels: easyfix, easytest > Fix For: 1.2.2 > > Attachments: HBASE-16469.git3f671c1ead.001.patch > > > *method invocation replaced by variable* > hbase-1.2.2/hbase-server/src/main/java/org/apache/hadoop/hbase/backup/example/LongTermArchivingHFileCleaner.java > line 57: {code}Path file = fStat.getPath();{code} > line 74: {code}LOG.error("Failed to lookup status of:" + fStat.getPath() + ", > keeping it just incase.", e); {code} > hbase-1.2.2/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/handler/CloseRegionHandler.java > line 118: {code}String name = regionInfo.getRegionNameAsString();{code} > line 142: {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-1.2.2/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java > line 5455: > {code} > public String toString() { > return getRegionInfo().getRegionNameAsString(); > } > {code} > line 1260: > {code} > LOG.debug("Region " + getRegionInfo().getRegionNameAsString() > + " is not mergeable because it is closing or closed"); > {code} > line 1265: > {code} > LOG.debug("Region " + getRegionInfo().getRegionNameAsString() > + " is not mergeable because it has references"); > {code} > line 1413: > {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-1.2.2/hbase-it/src/test/java/org/apache/hadoop/hbase/HBaseClusterManager.java > line 88: > {code} > if ((sshUserName != null && sshUserName.length() > 0) || > (sshOptions != null && sshOptions.length() > 0)) { > LOG.info("Running with SSH user [" + sshUserName + "] and options [" + > sshOptions + "]"); > } > {code} > hbase-1.2.2/hbase-server/src/main/java/org/apache/hadoop/hbase/master/AssignmentManager.java > line 980: > {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-1.2.2/hbase-server/src/test/java/org/apache/hadoop/hbase/util/MultiThreadedUpdater.java > line 145: > {code} > byte[] rowKey = dataGenerator.getDeterministicUniqueKey(rowKeyBase); > {code} > line 184: > {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* > hbase-1.2.2/hbase-client/src/main/java/org/apache/hadoop/hbase/client/AsyncProcess.java > {code} > LOG.warn("#" + id + ", the task was rejected by the pool. This is > unexpected."+ " Server is "+ server.getServerName(),t); > {code} > server is an instance of class ServerName, we found ServerName.java: > hbase-client/src/main/java/org/apache/hadoop/hbase/ServerName.java > {code} > @Override > public String toString() { > return getServerName(); > } > {code} > the toString method returns getServerName(), so the "server.getServerName()" > should be replaced with "server" in case of simplicity and readability > Similar examples are in: > hbase-1.2.2/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} > hbase-1.2.2/hbase-server/src/main/java/org/apache/hadoop/hbase/ipc/RpcServer.java > line 705: > {code} LOG.debug(getName() + ": disconnecting client " + c.getHostAddress()); > {code} > line 1259: > {code} > public String toString() { > return getHostAddress() + ":" + remotePort; > } > {code} -- This message was sent by Atlassian JIRA (v6.3.4#6332)