[GitHub] [geode] kohlmu-pivotal commented on a change in pull request #5758: GEODE-8721: member that should become coordinator never detects loss of current coordinator

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

kohlmu-pivotal commented on a change in pull request #5758:
URL: https://github.com/apache/geode/pull/5758#discussion_r525669951



##########
File path: geode-membership/src/main/java/org/apache/geode/distributed/internal/membership/gms/fd/GMSHealthMonitor.java
##########
@@ -152,7 +152,7 @@
   /**
    * Members undergoing final checks
    */
-  private final List<ID> membersInFinalCheck =
+  final List<ID> membersInFinalCheck =

Review comment:
       do we need to add `@VisibleForTesting` annotation here?

##########
File path: geode-membership/src/main/java/org/apache/geode/distributed/internal/membership/gms/fd/GMSHealthMonitor.java
##########
@@ -130,7 +130,7 @@
   public static final long MEMBER_SUSPECT_COLLECTION_INTERVAL =
       Long.getLong("geode.suspect-member-collection-interval", 200);
 
-  private volatile long currentTimeStamp;
+  volatile long currentTimeStamp;

Review comment:
       Should we maybe remove direct access to this field and rather replace it with an accessor that we can then mark with `@VisibleForTesting`?




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