jdeppe-pivotal commented on a change in pull request #5778:
URL: https://github.com/apache/geode/pull/5778#discussion_r541439516
##########
File path:
geode-core/src/main/java/org/apache/geode/management/internal/FederatingManager.java
##########
@@ -380,8 +384,10 @@ void addMemberArtifacts(InternalDistributedMember member) {
return;
}
- try {
+ FederatingManagerCancelCriterion cancelCriterion =
Review comment:
I'm really not familiar with the usage of `CancelCriterion` so perhaps
my understanding of what's required here is a bit naive... My understanding was
simply that we didn't want to indefinitely block any threads `await`ing in
either of the two listeners in question here. That situation would arise if
something bad happened in the `FederatingManager` before it could get to
calling `readyForEvents`. Hence the use of the `StoppableCountdownLatch`.
Since what's calling into these listeners is a callback using an
asynchronous thread, I don't understand why it is the responsibility of these
listeners to go through all of this ceremony to tell some random component that
the cache is closing. Surely that component should have its own means of
determining that the cache is closing? Why is it not enough for the thread to
be released and throw an exception?
----------------------------------------------------------------
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]
