[GitHub] [geode] Bill commented on a change in pull request #6298: GEODE-9128: Remove host name look-up from JGAddress

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

Bill commented on a change in pull request #6298:
URL: https://github.com/apache/geode/pull/6298#discussion_r612044863



##########
File path: geode-membership/src/main/java/org/apache/geode/distributed/internal/membership/gms/messenger/JGAddress.java
##########
@@ -91,9 +91,9 @@ public String toString() {
     StringBuilder sb = new StringBuilder();
 
     if (ip_addr == null)
-      sb.append("<null>");
+      sb.append("<no address>");
     else {
-      sb.append(ip_addr.getHostName());
+      sb.append(ip_addr);

Review comment:
       yes

##########
File path: geode-membership/src/main/java/org/apache/geode/distributed/internal/membership/gms/messenger/JGroupsMessenger.java
##########
@@ -335,7 +335,9 @@ public void start() throws MemberStartupException {
     try {
       Object oldDSMembershipInfo = services.getConfig().getOldMembershipInfo();
       if (oldDSMembershipInfo != null) {
-        logger.debug("Reusing JGroups channel from previous system", properties);
+        if (logger.isDebugEnabled()) {

Review comment:
       This extra conditional (and the others like it) don't seem to improve performance or maintainability. Is there a coding standard for Geode, that necessitates this pre-checking?
   
   I searched the codebase for e.g. "logger.debug(" calls and while many do the pre-check like this, many others do not.




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