sodonnel commented on a change in pull request #3820:
URL: https://github.com/apache/hadoop/pull/3820#discussion_r772319216
##########
File path:
hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/router/NamenodeHeartbeatService.java
##########
@@ -272,7 +272,7 @@ private void updateState() {
} else if (localTarget == null) {
// block info available, HA status not expected
LOG.debug(
- "Reporting non-HA namenode as operational: " + getNamenodeDesc());
+ "Reporting non-HA namenode as operational: {}", getNamenodeDesc());
Review comment:
I think this statement really needs wrapped in an if
`(LOG.isDebugEnabled())`. The reason is that the method call
`getNamenodeDesc()` needs to be evaluated whether we log or not, so its result
can be passed into the `LOG.debug()` method. Inside `LOG.debug`, it will skip
the logging, but by then, we have already formed the string inside
`getNamenodeDesc()` and never use it.
If we are passing an object into `LOG.debug`, this change would be the
correct thing to do, as toString() will only get called on the object if the
log message is created. However when we pass a method call into the method, it
needs to be evaluated first AFAIK.
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
