dsmiley commented on code in PR #2571:
URL: https://github.com/apache/solr/pull/2571#discussion_r1687153189
##########
solr/solrj/src/java/org/apache/solr/client/solrj/impl/CloudSolrClient.java:
##########
@@ -1213,10 +1193,6 @@ protected DocCollection getDocCollection(String
collection, Integer expectedVers
// no such collection exists
return null;
}
- if (!ref.isLazilyLoaded()) {
Review Comment:
In summary, by bailing early here, we never even populated the cache. The
cache may not be important for a the ZK CSP because that has its own cache via
ZkStateReader holding and always updating the ClusterState. But this isn't
true of HTTP CSP and I don't think layering another cache there is the ideal
approach either. The "stateVer" protocol in place makes sense; embraces an
eventually-consistent mindset instead of pretending everything everywhere knows
all.
By removing this check we'll get a _cached_ DocCollection, even for the ZK
CSP which has access to the latest state (albeit eventually consistent). This
is kind of sad, removing a benefit of the ZK CSP in that it's routing is
extremely closely up to date.
Perhaps instead, put this back and modify HttpClusterStateProvider to return
a ClusterState.CollectionRef subclass that returns "true" for isLazilyLoaded,
and that which actually fetches the state in get() (thus lazily not eagerly)?
--
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: issues-unsubscr...@solr.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org
For additional commands, e-mail: issues-h...@solr.apache.org
