[GitHub] [geode] kirklund commented on a diff in pull request #6225: GEODE-9101: fix VisibleNode attribute in MemberMXBean

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

kirklund commented on code in PR #6225:
URL: https://github.com/apache/geode/pull/6225#discussion_r845355835


##########
geode-core/src/main/java/org/apache/geode/distributed/internal/ClusterDistributionManager.java:
##########
@@ -350,13 +350,13 @@ static ClusterDistributionManager create(InternalDistributedSystem system,
               }
             }
           }
+        } else {
+          distributionManager.addNewMember(id); // add ourselves

Review Comment:
   We probably need some unit tests for ClusterDistributionManager which would be closer to the actual code than JMX.
   
   I can't sign off on this change for now though because as far as I know the value of `nodes` is correct. I still think you should convince dev-list that the value of `nodes` is incorrect or you should leave the value alone.



##########
geode-core/src/main/java/org/apache/geode/distributed/internal/ClusterDistributionManager.java:
##########
@@ -1793,9 +1793,7 @@ private String prettifyReason(String r) {
    */
   private void handleManagerStartup(InternalDistributedMember theId) {
     // Note test is under membersLock
-    if (theId.getVmKind() != ClusterDistributionManager.LOCATOR_DM_TYPE) {
-      stats.incNodes(1);
-    }
+    stats.incNodes(1);

Review Comment:
   Why is it ok to remove the if-condition from here?



-- 
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: notifications-unsubscribe@geode.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org