[jira] [Updated] (SOLR-8323) Add CollectionWatcher API to ZkStateReader

["Alan Woodward (JIRA)" <ji...@apache.org>]

     [ https://issues.apache.org/jira/browse/SOLR-8323?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Alan Woodward updated SOLR-8323:
--------------------------------
    Attachment: SOLR-8323.patch

Wow, thanks for the very thorough review Scott!  Here's an updated patch.

bq. Could collectionWatches and interestingCollections be unified into a single thing?

Unfortunately not, as it's needed to detect collections which have migrated from state format 1 to state format 2.  There's almost certainly a nicer way of doing that, though - maybe in a follow-up issue?

bq. make the static type of collectionWatchers be ConcurrentMap?

I disagree here - we don't use any of the concurrent methods, so I think just using Map is fine?

bq. unregisterCore needs a better guard against under-referencing

Added.  I don't think throwing an exception is necessary, although maybe we should log a warning in this case?

bq. newState is always null

changed

bq. No need to early exit here

changed

bq. In notifyStateWatchers you can avoid some copies...

I think this ends up as a wash, given that you may end up creating multiple HashSets?  And we're only copying references, after all.

bq. In getStateWatchers() you probably still want to wrap in a compute function...

Compute() doesn't help here, I don't think?  And given that it's a test-only method, I'm not too concerned about accuracy.  I've made it return a copy rather than return the original set, which should stop weird things happening to it once it's been returned, though.

bq. fetchCollectionState() expectExists parameter doesn't make sense to me

Again, this is due to state format 1 - a collection state might be in clusterstate.json, so the collection-specific state might not exist.  I agree about the null watcher though, and have added a check around the exists call for that.

bq. registerCore/ unregisterCore should probably retain the previous doc:

Added back

bq. getStateWatchers() could return null vs. empty set

Nice idea, added.

I've also added an explicit test for state format 1 collections, and updated the code so that it actually works :)

> Add CollectionWatcher API to ZkStateReader
> ------------------------------------------
>
>                 Key: SOLR-8323
>                 URL: https://issues.apache.org/jira/browse/SOLR-8323
>             Project: Solr
>          Issue Type: Improvement
>    Affects Versions: master
>            Reporter: Alan Woodward
>            Assignee: Alan Woodward
>         Attachments: SOLR-8323.patch, SOLR-8323.patch, SOLR-8323.patch, SOLR-8323.patch
>
>
> An API to watch for changes to collection state would be a generally useful thing, both internally and for client use.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org