dsmiley commented on code in PR #2363:
URL: https://github.com/apache/solr/pull/2363#discussion_r1534052101
##########
solr/core/src/java/org/apache/solr/servlet/HttpSolrCall.java:
##########
@@ -345,6 +348,41 @@ protected void init() throws Exception {
action = PASSTHROUGH;
}
+ /**
+ * Lookup the collection from the collection string (maybe comma delimited).
Also sets {@link
+ * #collectionsList} by side-effect. if {@code secondTry} is false then
we'll potentially
+ * recursively try this all one more time while ensuring the alias and
collection info is sync'ed
+ * from ZK.
+ */
+ protected DocCollection resolveDocCollection(List<String> collectionsList) {
+ if (!cores.isZooKeeperAware()) {
+ throw new SolrException(
+ SolrException.ErrorCode.BAD_REQUEST, "Solr not running in cloud mode
");
+ }
+ ZkStateReader zkStateReader = cores.getZkController().getZkStateReader();
+ Supplier<DocCollection> logic =
+ () -> {
+ String collectionName = collectionsList.get(0); // first
+ // TODO an option to choose another collection in the list if can't
find a local replica
+ // of the first?
+ return
zkStateReader.getClusterState().getCollectionOrNull(collectionName);
+ };
+
+ DocCollection docCollection = logic.get();
+ if (docCollection != null) {
+ return docCollection;
+ }
+ // ensure our view is up to date before trying again
+ try {
+ zkStateReader.aliasesManager.update();
+ zkStateReader.forceUpdateCollection(collectionsList.get(0));
Review Comment:
again; the choice to do this for the first collection seems repeated here
(see earlier `collectionsList.get(0)`). Would be clearer to do this once and
have a var like "collectionName".
##########
solr/core/src/java/org/apache/solr/servlet/HttpSolrCall.java:
##########
@@ -345,6 +348,41 @@ protected void init() throws Exception {
action = PASSTHROUGH;
}
+ /**
+ * Lookup the collection from the collection string (maybe comma delimited).
Also sets {@link
+ * #collectionsList} by side-effect. if {@code secondTry} is false then
we'll potentially
+ * recursively try this all one more time while ensuring the alias and
collection info is sync'ed
+ * from ZK.
+ */
+ protected DocCollection resolveDocCollection(List<String> collectionsList) {
+ if (!cores.isZooKeeperAware()) {
+ throw new SolrException(
+ SolrException.ErrorCode.BAD_REQUEST, "Solr not running in cloud mode
");
+ }
+ ZkStateReader zkStateReader = cores.getZkController().getZkStateReader();
+ Supplier<DocCollection> logic =
+ () -> {
+ String collectionName = collectionsList.get(0); // first
+ // TODO an option to choose another collection in the list if can't
find a local replica
+ // of the first?
Review Comment:
I know you are just moving code but these 3 lines seem like they don't
belong inside the supplier; should be done before hand. Won't need to be
repeated.
##########
solr/solrj-zookeeper/src/java/org/apache/solr/client/solrj/impl/ZkClientClusterStateProvider.java:
##########
@@ -144,11 +144,22 @@ public static ClusterState
createFromJsonSupportingLegacyConfigName(
@Override
public ClusterState.CollectionRef getState(String collection) {
ClusterState clusterState = getZkStateReader().getClusterState();
- if (clusterState != null) {
- return clusterState.getCollectionRef(collection);
- } else {
+ if (clusterState == null) {
Review Comment:
When would this happen? If it's a startup/initialization scenario, maybe
getClusterState() should wait until it exists? Okay to defer this question to
another PR as this concern pre-existed albeit maybe it could be another reason
why we aren't resolving the collection.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
