kirklund commented on a change in pull request #6225:
URL: https://github.com/apache/geode/pull/6225#discussion_r609909199
##########
File path:
geode-core/src/main/java/org/apache/geode/distributed/internal/ClusterDistributionManager.java
##########
@@ -1881,9 +1881,8 @@ public void
handleManagerDeparture(InternalDistributedMember theId, boolean memb
removeHostedLocators(theId);
redundancyZones.remove(theId);
- if (theId.getVmKind() != ClusterDistributionManager.LOCATOR_DM_TYPE) {
- stats.incNodes(-1);
- }
+ stats.incNodes(-1);
+
Review comment:
I understand the purpose of the PR better now. However, the PR is still
trying to redefine the "nodes" statistic in DistributionStats which is not
correct. You should instead add a new attribute to MemberMXBean that returns
the count of OTHER members in the cluster.
Here is the definition of the "nodes" statistic:
```
final String nodesDesc = "The current number of nodes in this
distributed system.";
```
So, if you have 2 servers and 2 locators, "nodes" should be 4 within all 4
JVMs.
The earliest version of GemFire had locators which were not members (ie not
nodes), but locators now are definitely members (ie nodes). The next type of
locators in GemFire were members (ie nodes) but without caches. As of GemFire
7.0, all locators also have a cache. So in every version of Geode, all locators
are members with caches. The methods to create that earliest type of locator
still exists but are deprecated... so deprecated internal methods is now the
only way to create the thin locators that are not peer-to-peer members of the
cluster.
Changing the attribute of an MXBean is _different_ from changing the meaning
of the statistic. The statistic is not supposed to ignore locators.
--
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:
[email protected]
