jchen21 commented on code in PR #7670:
URL: https://github.com/apache/geode/pull/7670#discussion_r869675129
##########
geode-core/src/main/java/org/apache/geode/internal/cache/InitialImageOperation.java:
##########
@@ -1077,7 +1081,8 @@ protected Set<Object>
processReceivedRVV(RegionVersionVector remoteRVV,
if (id == null) {
id = myId;
}
- if (!remoteRVV.contains(id, stamp.getRegionVersion())) {
+ foundIds.add(id);
Review Comment:
This will add `id` to `foundIds`, even for the persistent case. However,
according to the description of GEODE-10290, it is for the non-persistent case:
>In non-persistent but concurrent-check enabled case, members departed will
be marked. They should be removed from RVV during GII to prevent
memberToVersion in RVV grows bigger and bigger.
So I am not sure if we should add a `if` check here. e.g. `if
(!isPersistentRegion)` .
##########
geode-core/src/main/java/org/apache/geode/internal/cache/InitialImageOperation.java:
##########
@@ -1100,6 +1105,12 @@ protected Set<Object>
processReceivedRVV(RegionVersionVector remoteRVV,
}
}
}
+ if (foundIds.size() > 0) {
Review Comment:
`foundIds` always has at least one element. Because of line 1067
`foundIds.add(region.getVersionMember());`. So this `if ` check is not
necessary.
--
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-unsubscr...@geode.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
