dschneider-pivotal commented on a change in pull request #6093:
URL: https://github.com/apache/geode/pull/6093#discussion_r588611334
##########
File path:
geode-core/src/main/java/org/apache/geode/internal/cache/persistence/PersistenceAdvisorImpl.java
##########
@@ -533,7 +535,8 @@ public boolean
checkMyStateOnMembers(Set<InternalDistributedMember> replicates)
String message = String.format(
"Region %s remote member %s with persistent data %s was not part
of the same distributed system as the local data from %s",
regionPath, member, remoteId, myId);
- throw new ConflictingPersistentDataException(message);
+ logger.warn(message);
+ iterator.remove();
Review comment:
I don't understand why you are going to the trouble of doing this remove.
This remove will modify the HashMap returned by
remoteStates.getStateOnPeers(). This is the field "stateOnPeers" in
PersistentStateQueryResults. But this instance of PersistentStateQueryResults
is created in this method on line 512: PersistentStateQueryResults
remoteStates = getMyStateOnMembers(replicates);
So it seems to me like no other thread will have this same instance.
fetchPersistentStateQueryResults always sends a message and then this instance
comes back as the result. So I can't find any other code that would ever see
this entry that you are removing. So it seems to me like this entire PR could
just be not to throw the exception. If you get rid of the remove then you don't
need the iterator change. Am I missing something?
----------------------------------------------------------------
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]