Github user dragonsinth commented on the pull request:
https://github.com/apache/lucene-solr/pull/32#issuecomment-216333963
@romseygeek nice job on the changes so far, and sorry to have so much
feedback and so many asks. This is a pretty complicated change so I feel like
it merits the attention to detail.
I feel like we're at a fork in the road with this patch at the moment
though, and we need to get more people involved to proceed. Let me explain.
Even having fixed the "calling watchers while holding locks issue", the one
thing that makes me most nervous about the current state is that we're still
potentially executing user-provided predicates on threads that belong to a
variety of other people-- e.g. the caller of forceUpdateCollection() or even
the Zk event callback thread. We could make a tactical fix to the
implementation of waitForState() by turning that method into a loop and running
the predicate on the actual thread that called waitForState(), such that the
onStateChanged() handler doesn't dip into client code.
But honestly, I feel like having privatized CollectionStateWatcher and the
ability to register / unregister is a missed opportunity. I can think of uses
for the feature, like in some cases Overseer operations could watch a
collection for the duration of an operation to prevent having to re-query ZK.
To make that solid, we'd need to either introduce an Executor in ZkStateReader
for publishing events, or else require the watch registration to provide an
executor, the way Guava's ListenableFuture does.
Thoughts? I'd also like to hear from others.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org
