From e001f352895c83652c3cf31e3c724d29a46bb721 Mon Sep 17 00:00:00 2001 From: David Smiley Date: Thu, 19 Oct 2017 00:02:24 -0400 Subject: [PATCH] SOLR-11444: Improve consistency of collection alias handling and collection list references. Other refactorings of nearby code too. --- solr/CHANGES.txt | 5 + .../java/org/apache/solr/api/V2HttpCall.java | 36 ++- .../org/apache/solr/cloud/CreateAliasCmd.java | 52 ++-- .../org/apache/solr/cloud/DeleteAliasCmd.java | 24 +- .../solr/handler/admin/ClusterStatus.java | 34 +-- .../apache/solr/handler/sql/SolrSchema.java | 6 +- .../search/join/ScoreJoinQParserPlugin.java | 30 +-- .../org/apache/solr/servlet/HttpSolrCall.java | 248 ++++++++---------- .../solr/cloud/AliasIntegrationTest.java | 146 ++++++----- .../cloud/DistribJoinFromCollectionTest.java | 9 +- .../DelegatingClusterStateProvider.java | 16 +- .../client/solrj/impl/CloudSolrClient.java | 197 ++++++-------- .../solrj/impl/ClusterStateProvider.java | 12 +- .../solrj/impl/HttpClusterStateProvider.java | 20 +- .../impl/ZkClientClusterStateProvider.java | 19 +- .../solrj/io/stream/CloudSolrStream.java | 38 ++- .../client/solrj/io/stream/TupleStream.java | 44 +--- .../response/CollectionAdminResponse.java | 7 + .../org/apache/solr/common/cloud/Aliases.java | 129 +++++++-- .../solr/common/cloud/ClusterState.java | 9 - .../solr/common/cloud/ZkStateReader.java | 10 +- .../org/apache/solr/common/util/StrUtils.java | 1 + .../solrj/impl/CloudSolrClientCacheTest.java | 10 +- .../solr/client/solrj/io/sql/JdbcTest.java | 9 +- .../java/org/apache/solr/SolrTestCaseJ4.java | 32 ++- 25 files changed, 526 insertions(+), 617 deletions(-) diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt index 2715fe50079..374e235c94b 100644 --- a/solr/CHANGES.txt +++ b/solr/CHANGES.txt @@ -79,6 +79,11 @@ Other Changes * SOLR-11464, SOLR-11493: Minor refactorings to DistributedUpdateProcessor. (Gus Heck, David Smiley) +* SOLR-11444: Improved consistency of collection alias handling, and collection references that are + comma delimited lists of collections, across the various places collections can be referred to. + Updates are delivered to the first in a list. It's now possible to refer to a comma delimited list + of them in the path, ex: /solr/colA,colB/select?... (David Smiley) + ================== 7.1.0 ================== Consult the LUCENE_CHANGES.txt file for additional, low level, changes in this release. diff --git a/solr/core/src/java/org/apache/solr/api/V2HttpCall.java b/solr/core/src/java/org/apache/solr/api/V2HttpCall.java index ab81f76785b..d712289f1a8 100644 --- a/solr/core/src/java/org/apache/solr/api/V2HttpCall.java +++ b/solr/core/src/java/org/apache/solr/api/V2HttpCall.java @@ -53,6 +53,7 @@ import org.apache.solr.common.util.PathTrie; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import static org.apache.solr.common.cloud.ZkStateReader.COLLECTION_PROP; import static org.apache.solr.common.params.CommonParams.JSON; import static org.apache.solr.common.params.CommonParams.WT; import static org.apache.solr.servlet.SolrDispatchFilter.Action.ADMIN; @@ -105,28 +106,30 @@ public class V2HttpCall extends HttpSolrCall { } if ("c".equals(prefix) || "collections".equals(prefix)) { - String collectionName = origCorename = corename = pieces.get(1); + origCorename = pieces.get(1); + + collectionsList = resolveCollectionListOrAlias(queryParams.get(COLLECTION_PROP, origCorename)); + String collectionName = collectionsList.get(0); // first + //TODO try the other collections if can't find a local replica of the first? + DocCollection collection = getDocCollection(collectionName); if (collection == null) { - if ( ! path.endsWith(CommonParams.INTROSPECT)) { + if ( ! path.endsWith(CommonParams.INTROSPECT)) { throw new SolrException(SolrException.ErrorCode.BAD_REQUEST, "no such collection or alias"); } } else { - boolean isPreferLeader = false; - if (path.endsWith("/update") || path.contains("/update/")) { - isPreferLeader = true; - } + boolean isPreferLeader = (path.endsWith("/update") || path.contains("/update/")); core = getCoreByCollection(collection.getName(), isPreferLeader); if (core == null) { //this collection exists , but this node does not have a replica for that collection //todo find a better way to compute remote - extractRemotePath(corename, origCorename, 0); + extractRemotePath(collectionName, origCorename, 0); return; } } } else if ("cores".equals(prefix)) { - origCorename = corename = pieces.get(1); - core = cores.getCore(corename); + origCorename = pieces.get(1); + core = cores.getCore(origCorename); } if (core == null) { log.error(">> path: '" + path + "'"); @@ -134,7 +137,7 @@ public class V2HttpCall extends HttpSolrCall { initAdminRequest(path); return; } else { - throw new SolrException(SolrException.ErrorCode.NOT_FOUND, "no core retrieved for " + corename); + throw new SolrException(SolrException.ErrorCode.NOT_FOUND, "no core retrieved for " + origCorename); } } @@ -148,9 +151,7 @@ public class V2HttpCall extends HttpSolrCall { MDCLoggingContext.setCore(core); parseRequest(); - if (usingAliases) { - processAliases(aliases, collectionsList); - } + addCollectionParamIfNeeded(getCollectionsList()); action = PROCESS; // we are done with a valid handler @@ -180,17 +181,12 @@ public class V2HttpCall extends HttpSolrCall { if (solrReq == null) solrReq = parser.parse(core, path, req); } - protected DocCollection getDocCollection(String collectionName) { + protected DocCollection getDocCollection(String collectionName) { // note: don't send an alias; resolve it first if (!cores.isZooKeeperAware()) { throw new SolrException(SolrException.ErrorCode.BAD_REQUEST, "Solr not running in cloud mode "); } ZkStateReader zkStateReader = cores.getZkController().getZkStateReader(); - DocCollection collection = zkStateReader.getClusterState().getCollectionOrNull(collectionName); - if (collection == null) { - collectionName = corename = lookupAliases(collectionName); - collection = zkStateReader.getClusterState().getCollectionOrNull(collectionName); - } - return collection; + return zkStateReader.getClusterState().getCollectionOrNull(collectionName); } public static Api getApiInfo(PluginBag requestHandlers, diff --git a/solr/core/src/java/org/apache/solr/cloud/CreateAliasCmd.java b/solr/core/src/java/org/apache/solr/cloud/CreateAliasCmd.java index 339d75ac02a..16a4266d6a1 100644 --- a/solr/core/src/java/org/apache/solr/cloud/CreateAliasCmd.java +++ b/solr/core/src/java/org/apache/solr/cloud/CreateAliasCmd.java @@ -17,27 +17,28 @@ */ package org.apache.solr.cloud; -import static org.apache.solr.common.params.CommonParams.NAME; - import java.lang.invoke.MethodHandles; -import java.util.HashMap; +import java.util.HashSet; +import java.util.List; import java.util.Locale; -import java.util.Map; +import java.util.Objects; +import java.util.Set; import java.util.concurrent.TimeUnit; import org.apache.solr.cloud.OverseerCollectionMessageHandler.Cmd; import org.apache.solr.common.SolrException; -import org.apache.solr.common.cloud.Aliases; import org.apache.solr.common.cloud.ClusterState; import org.apache.solr.common.cloud.ZkNodeProps; import org.apache.solr.common.cloud.ZkStateReader; import org.apache.solr.common.util.NamedList; -import org.apache.solr.common.util.Utils; +import org.apache.solr.common.util.StrUtils; import org.apache.solr.util.TimeOut; import org.apache.zookeeper.KeeperException; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import static org.apache.solr.common.params.CommonParams.NAME; + public class CreateAliasCmd implements Cmd { private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass()); @@ -51,24 +52,12 @@ public class CreateAliasCmd implements Cmd { public void call(ClusterState state, ZkNodeProps message, NamedList results) throws Exception { String aliasName = message.getStr(NAME); - String collections = message.getStr("collections"); + String collections = message.getStr("collections"); // could be comma delimited list ZkStateReader zkStateReader = ocmh.zkStateReader; - Map prevColAliases = zkStateReader.getAliases().getCollectionAliasMap(); - validateAllCollectionsExist(collections, prevColAliases, zkStateReader.getClusterState()); + validateAllCollectionsExistAndNoDups(collections, zkStateReader); - Map> newAliasesMap = new HashMap<>(); - Map newCollectionAliasesMap = new HashMap<>(); - if (prevColAliases != null) { - newCollectionAliasesMap.putAll(prevColAliases); - } - newCollectionAliasesMap.put(aliasName, collections); - newAliasesMap.put("collection", newCollectionAliasesMap); - Aliases newAliases = new Aliases(newAliasesMap); - byte[] jsonBytes = null; - if (newAliases.collectionAliasSize() > 0) { // only sub map right now - jsonBytes = Utils.toJSON(newAliases.getAliasMap()); - } + byte[] jsonBytes = zkStateReader.getAliases().cloneWithCollectionAlias(aliasName, collections).toJSON(); try { zkStateReader.getZkClient().setData(ZkStateReader.ALIASES, jsonBytes, true); @@ -84,10 +73,16 @@ public class CreateAliasCmd implements Cmd { } } - private void validateAllCollectionsExist(String collections, Map prevColAliases, ClusterState clusterState) { - String[] collectionArr = collections.split(","); - for (String collection:collectionArr) { - if (clusterState.getCollectionOrNull(collection) == null && (prevColAliases == null || !prevColAliases.containsKey(collection))) { + private void validateAllCollectionsExistAndNoDups(String collections, ZkStateReader zkStateReader) { + List collectionArr = StrUtils.splitSmart(collections, ",", true); + if (new HashSet<>(collectionArr).size() != collectionArr.size()) { + throw new SolrException(SolrException.ErrorCode.BAD_REQUEST, + String.format(Locale.ROOT, "Can't create collection alias for collections='%s', since it contains duplicates", collections)); + } + ClusterState clusterState = zkStateReader.getClusterState(); + Set aliasNames = zkStateReader.getAliases().getCollectionAliasListMap().keySet(); + for (String collection : collectionArr) { + if (clusterState.getCollectionOrNull(collection) == null && !aliasNames.contains(collection)) { throw new SolrException(SolrException.ErrorCode.BAD_REQUEST, String.format(Locale.ROOT, "Can't create collection alias for collections='%s', '%s' is not an existing collection or alias", collections, collection)); } @@ -95,14 +90,11 @@ public class CreateAliasCmd implements Cmd { } private void checkForAlias(String name, String value) { - TimeOut timeout = new TimeOut(30, TimeUnit.SECONDS); boolean success = false; - Aliases aliases; while (!timeout.hasTimedOut()) { - aliases = ocmh.zkStateReader.getAliases(); - String collections = aliases.getCollectionAlias(name); - if (collections != null && collections.equals(value)) { + String collections = ocmh.zkStateReader.getAliases().getCollectionAliasMap().get(name); + if (Objects.equals(collections, value)) { success = true; break; } diff --git a/solr/core/src/java/org/apache/solr/cloud/DeleteAliasCmd.java b/solr/core/src/java/org/apache/solr/cloud/DeleteAliasCmd.java index 7b1993ce4a0..4743cb9ff0d 100644 --- a/solr/core/src/java/org/apache/solr/cloud/DeleteAliasCmd.java +++ b/solr/core/src/java/org/apache/solr/cloud/DeleteAliasCmd.java @@ -19,17 +19,13 @@ package org.apache.solr.cloud; import java.lang.invoke.MethodHandles; -import java.util.HashMap; -import java.util.Map; import java.util.concurrent.TimeUnit; import org.apache.solr.common.SolrException; -import org.apache.solr.common.cloud.Aliases; import org.apache.solr.common.cloud.ClusterState; import org.apache.solr.common.cloud.ZkNodeProps; import org.apache.solr.common.cloud.ZkStateReader; import org.apache.solr.common.util.NamedList; -import org.apache.solr.common.util.Utils; import org.apache.solr.util.TimeOut; import org.apache.zookeeper.KeeperException; import org.slf4j.Logger; @@ -49,20 +45,11 @@ public class DeleteAliasCmd implements OverseerCollectionMessageHandler.Cmd { public void call(ClusterState state, ZkNodeProps message, NamedList results) throws Exception { String aliasName = message.getStr(NAME); - Map> newAliasesMap = new HashMap<>(); - Map newCollectionAliasesMap = new HashMap<>(); ZkStateReader zkStateReader = ocmh.zkStateReader; - newCollectionAliasesMap.putAll(zkStateReader.getAliases().getCollectionAliasMap()); - newCollectionAliasesMap.remove(aliasName); - newAliasesMap.put("collection", newCollectionAliasesMap); - Aliases newAliases = new Aliases(newAliasesMap); - byte[] jsonBytes = null; - if (newAliases.collectionAliasSize() > 0) { // only sub map right now - jsonBytes = Utils.toJSON(newAliases.getAliasMap()); - } + byte[] jsonBytes = zkStateReader.getAliases().cloneWithCollectionAlias(aliasName, null).toJSON(); try { - zkStateReader.getZkClient().setData(ZkStateReader.ALIASES, - jsonBytes, true); + zkStateReader.getZkClient().setData(ZkStateReader.ALIASES, jsonBytes, true); + checkForAliasAbsence(aliasName); // some fudge for other nodes Thread.sleep(100); @@ -76,13 +63,10 @@ public class DeleteAliasCmd implements OverseerCollectionMessageHandler.Cmd { } private void checkForAliasAbsence(String name) { - TimeOut timeout = new TimeOut(30, TimeUnit.SECONDS); boolean success = false; - Aliases aliases = null; while (! timeout.hasTimedOut()) { - aliases = ocmh.zkStateReader.getAliases(); - String collections = aliases.getCollectionAlias(name); + String collections = ocmh.zkStateReader.getAliases().getCollectionAliasMap().get(name); if (collections == null) { success = true; break; diff --git a/solr/core/src/java/org/apache/solr/handler/admin/ClusterStatus.java b/solr/core/src/java/org/apache/solr/handler/admin/ClusterStatus.java index dbe2e790f9d..63c1a078fd2 100644 --- a/solr/core/src/java/org/apache/solr/handler/admin/ClusterStatus.java +++ b/solr/core/src/java/org/apache/solr/handler/admin/ClusterStatus.java @@ -38,20 +38,18 @@ import org.apache.solr.common.cloud.ZkStateReader; import org.apache.solr.common.params.ShardParams; import org.apache.solr.common.util.NamedList; import org.apache.solr.common.util.SimpleOrderedMap; -import org.apache.solr.common.util.StrUtils; import org.apache.solr.common.util.Utils; import org.apache.zookeeper.KeeperException; public class ClusterStatus { private final ZkStateReader zkStateReader; - private final String collection; - private ZkNodeProps message; + private final ZkNodeProps message; + private final String collection; // maybe null public ClusterStatus(ZkStateReader zkStateReader, ZkNodeProps props) { this.zkStateReader = zkStateReader; this.message = props; collection = props.getStr(ZkStateReader.COLLECTION_PROP); - } @SuppressWarnings("unchecked") @@ -60,20 +58,14 @@ public class ClusterStatus { // read aliases Aliases aliases = zkStateReader.getAliases(); Map> collectionVsAliases = new HashMap<>(); - Map aliasVsCollections = aliases.getCollectionAliasMap(); - if (aliasVsCollections != null) { - for (Map.Entry entry : aliasVsCollections.entrySet()) { - List colls = StrUtils.splitSmart(entry.getValue(), ','); - String alias = entry.getKey(); - for (String coll : colls) { - if (collection == null || collection.equals(coll)) { - List list = collectionVsAliases.get(coll); - if (list == null) { - list = new ArrayList<>(); - collectionVsAliases.put(coll, list); - } - list.add(alias); - } + Map> aliasVsCollections = aliases.getCollectionAliasListMap(); + for (Map.Entry> entry : aliasVsCollections.entrySet()) { + String alias = entry.getKey(); + List colls = entry.getValue(); + for (String coll : colls) { + if (collection == null || collection.equals(coll)) { + List list = collectionVsAliases.computeIfAbsent(coll, k -> new ArrayList<>()); + list.add(alias); } } } @@ -158,8 +150,9 @@ public class ClusterStatus { } // add the alias map too - if (aliasVsCollections != null && !aliasVsCollections.isEmpty()) { - clusterStatus.add("aliases", aliasVsCollections); + Map collectionAliasMap = aliases.getCollectionAliasMap(); // comma delim + if (!collectionAliasMap.isEmpty()) { + clusterStatus.add("aliases", collectionAliasMap); } // add the roles map @@ -172,6 +165,7 @@ public class ClusterStatus { results.add("cluster", clusterStatus); } + /** * Get collection status from cluster state. * Can return collection status by given shard name. diff --git a/solr/core/src/java/org/apache/solr/handler/sql/SolrSchema.java b/solr/core/src/java/org/apache/solr/handler/sql/SolrSchema.java index 20d01f33b34..c1bb06547d7 100644 --- a/solr/core/src/java/org/apache/solr/handler/sql/SolrSchema.java +++ b/solr/core/src/java/org/apache/solr/handler/sql/SolrSchema.java @@ -58,10 +58,8 @@ class SolrSchema extends AbstractSchema { } Aliases aliases = zkStateReader.getAliases(); - if(aliases.collectionAliasSize() > 0) { - for (Map.Entry alias : aliases.getCollectionAliasMap().entrySet()) { - builder.put(alias.getKey(), new SolrTable(this, alias.getValue())); - } + for (String alias : aliases.getCollectionAliasListMap().keySet()) { + builder.put(alias, new SolrTable(this, alias)); } return builder.build(); diff --git a/solr/core/src/java/org/apache/solr/search/join/ScoreJoinQParserPlugin.java b/solr/core/src/java/org/apache/solr/search/join/ScoreJoinQParserPlugin.java index 6715fd88180..7e8dbabf862 100644 --- a/solr/core/src/java/org/apache/solr/search/join/ScoreJoinQParserPlugin.java +++ b/solr/core/src/java/org/apache/solr/search/join/ScoreJoinQParserPlugin.java @@ -17,7 +17,7 @@ package org.apache.solr.search.join; import java.io.IOException; -import java.util.Map; +import java.util.List; import java.util.Objects; import org.apache.lucene.index.DocValuesType; @@ -275,10 +275,9 @@ public class ScoreJoinQParserPlugin extends QParserPlugin { public static String getCoreName(final String fromIndex, CoreContainer container) { if (container.isZooKeeperAware()) { ZkController zkController = container.getZkController(); - final String resolved = - zkController.getClusterState().hasCollection(fromIndex) - ? fromIndex : resolveAlias(fromIndex, zkController); - if (resolved == null) { + final String resolved = resolveAlias(fromIndex, zkController); + // TODO DWS: no need for this since later, clusterState.getCollection will throw a reasonable error + if (!zkController.getClusterState().hasCollection(resolved)) { throw new SolrException(SolrException.ErrorCode.BAD_REQUEST, "SolrCloud join: Collection '" + fromIndex + "' not found!"); } @@ -289,21 +288,14 @@ public class ScoreJoinQParserPlugin extends QParserPlugin { private static String resolveAlias(String fromIndex, ZkController zkController) { final Aliases aliases = zkController.getZkStateReader().getAliases(); - if (aliases != null) { - final String resolved; - Map collectionAliases = aliases.getCollectionAliasMap(); - resolved = (collectionAliases != null) ? collectionAliases.get(fromIndex) : null; - if (resolved != null) { - if (resolved.split(",").length > 1) { - throw new SolrException(SolrException.ErrorCode.BAD_REQUEST, - "SolrCloud join: Collection alias '" + fromIndex + - "' maps to multiple collections (" + resolved + - "), which is not currently supported for joins."); - } - return resolved; - } + List collections = aliases.resolveAliases(fromIndex); // if not an alias, returns input + if (collections.size() != 1) { + throw new SolrException(SolrException.ErrorCode.BAD_REQUEST, + "SolrCloud join: Collection alias '" + fromIndex + + "' maps to multiple collections (" + collections + + "), which is not currently supported for joins."); } - return null; + return collections.get(0); } private static String findLocalReplicaForFromIndex(ZkController zkController, String fromIndex) { diff --git a/solr/core/src/java/org/apache/solr/servlet/HttpSolrCall.java b/solr/core/src/java/org/apache/solr/servlet/HttpSolrCall.java index 2a72649ab17..39c60e9dbca 100644 --- a/solr/core/src/java/org/apache/solr/servlet/HttpSolrCall.java +++ b/solr/core/src/java/org/apache/solr/servlet/HttpSolrCall.java @@ -30,8 +30,8 @@ import java.util.Collection; import java.util.Collections; import java.util.Enumeration; import java.util.HashMap; -import java.util.HashSet; import java.util.Iterator; +import java.util.LinkedHashSet; import java.util.List; import java.util.Map; import java.util.Random; @@ -72,7 +72,9 @@ import org.apache.solr.common.params.CommonParams; import org.apache.solr.common.params.MapSolrParams; import org.apache.solr.common.params.ModifiableSolrParams; import org.apache.solr.common.params.SolrParams; +import org.apache.solr.common.util.CommandOperation; import org.apache.solr.common.util.ContentStream; +import org.apache.solr.common.util.JsonSchemaValidator; import org.apache.solr.common.util.NamedList; import org.apache.solr.common.util.SimpleOrderedMap; import org.apache.solr.common.util.StrUtils; @@ -101,8 +103,6 @@ import org.apache.solr.servlet.SolrDispatchFilter.Action; import org.apache.solr.servlet.cache.HttpCacheHeaderUtil; import org.apache.solr.servlet.cache.Method; import org.apache.solr.update.processor.DistributingUpdateProcessorFactory; -import org.apache.solr.common.util.CommandOperation; -import org.apache.solr.common.util.JsonSchemaValidator; import org.apache.solr.util.RTimerTree; import org.apache.solr.util.TimeOut; import org.apache.zookeeper.KeeperException; @@ -160,13 +160,10 @@ public class HttpSolrCall { protected String coreUrl; protected SolrConfig config; protected Map invalidStates; - protected boolean usingAliases = false; //The states of client that is invalid in this request - protected Aliases aliases = null; - protected String corename = ""; - protected String origCorename = null; - + protected String origCorename; // What's in the URL path; might reference a collection/alias or a Solr core name + protected List collectionsList; // The list of SolrCloud collections if in SolrCloud (usually 1) public RequestType getRequestType() { return requestType; @@ -174,13 +171,6 @@ public class HttpSolrCall { protected RequestType requestType; - - public List getCollectionsList() { - return collectionsList; - } - - protected List collectionsList; - public HttpSolrCall(SolrDispatchFilter solrDispatchFilter, CoreContainer cores, HttpServletRequest request, HttpServletResponse response, boolean retry) { this.solrDispatchFilter = solrDispatchFilter; @@ -206,7 +196,6 @@ public class HttpSolrCall { return path; } - public HttpServletRequest getReq() { return req; } @@ -219,12 +208,22 @@ public class HttpSolrCall { return queryParams; } + protected Aliases getAliases() { + return cores.isZooKeeperAware() ? cores.getZkController().getZkStateReader().getAliases() : Aliases.EMPTY; + } + + /** The collection(s) referenced in this request. Populated in {@link #init()}. Not null. */ + public List getCollectionsList() { + return collectionsList != null ? collectionsList : Collections.emptyList(); + } + protected void init() throws Exception { // check for management path String alternate = cores.getManagementPath(); if (alternate != null && path.startsWith(alternate)) { path = path.substring(0, alternate.length()); } + // unused feature ? int idx = path.indexOf(':'); if (idx > 0) { @@ -232,8 +231,6 @@ public class HttpSolrCall { path = path.substring(0, idx); } - boolean usingAliases = false; - // Check for container handlers handler = cores.getRequestHandler(path); if (handler != null) { @@ -242,69 +239,59 @@ public class HttpSolrCall { requestType = RequestType.ADMIN; action = ADMIN; return; - } else { - //otherwise, we should find a core from the path - idx = path.indexOf("/", 1); - if (idx > 1) { - // try to get the corename as a request parameter first - corename = path.substring(1, idx); + } - // look at aliases - if (cores.isZooKeeperAware()) { - origCorename = corename; - ZkStateReader reader = cores.getZkController().getZkStateReader(); - aliases = reader.getAliases(); - if (aliases != null && aliases.collectionAliasSize() > 0) { - usingAliases = true; - String alias = aliases.getCollectionAlias(corename); - if (alias != null) { - collectionsList = StrUtils.splitSmart(alias, ",", true); - corename = collectionsList.get(0); - } - } + // Parse a core or collection name from the path and attempt to see if it's a core name + idx = path.indexOf("/", 1); + if (idx > 1) { + origCorename = path.substring(1, idx); + + // Try to resolve a Solr core name + core = cores.getCore(origCorename); + if (core != null) { + path = path.substring(idx); + } else { + if (cores.isCoreLoading(origCorename)) { // extra mem barriers, so don't look at this before trying to get core + throw new SolrException(ErrorCode.SERVICE_UNAVAILABLE, "SolrCore is loading"); } - - core = cores.getCore(corename); + // the core may have just finished loading + core = cores.getCore(origCorename); if (core != null) { path = path.substring(idx); - } else if (cores.isCoreLoading(corename)) { // extra mem barriers, so don't look at this before trying to get core - throw new SolrException(ErrorCode.SERVICE_UNAVAILABLE, "SolrCore is loading"); } else { - // the core may have just finished loading - core = cores.getCore(corename); - if (core != null) { - path = path.substring(idx); + if (!cores.isZooKeeperAware()) { + core = cores.getCore(""); } } } - if (core == null) { - if (!cores.isZooKeeperAware()) { - core = cores.getCore(""); - } - } } - if (core == null && cores.isZooKeeperAware()) { - // we couldn't find the core - lets make sure a collection was not specified instead - boolean isPreferLeader = false; - if (path.endsWith("/update") || path.contains("/update/")) { - isPreferLeader = true; - } - core = getCoreByCollection(corename, isPreferLeader); - if (core != null) { - // we found a core, update the path - path = path.substring(idx); - if (collectionsList == null) - collectionsList = new ArrayList<>(); - collectionsList.add(corename); - } + if (cores.isZooKeeperAware()) { + // init collectionList (usually one name but not when there are aliases) + String def = core != null ? core.getCoreDescriptor().getCollectionName() : origCorename; + collectionsList = resolveCollectionListOrAlias(queryParams.get(COLLECTION_PROP, def)); // &collection= takes precedence - // if we couldn't find it locally, look on other nodes - extractRemotePath(corename, origCorename, idx); - if (action != null) return; - //core is not available locally or remotely - autoCreateSystemColl(corename); - if(action != null) return; + if (core == null) { + // lookup core from collection, or route away if need to + String collectionName = collectionsList.isEmpty() ? null : collectionsList.get(0); // route to 1st + //TODO try the other collections if can't find a local replica of the first? (and do to V2HttpSolrCall) + + boolean isPreferLeader = (path.endsWith("/update") || path.contains("/update/")); + + core = getCoreByCollection(collectionName, isPreferLeader); // find a local replica/core for the collection + if (core != null) { + if (idx > 0) { + path = path.substring(idx); + } + } else { + // if we couldn't find it locally, look on other nodes + extractRemotePath(collectionName, origCorename, idx); + if (action != null) return; + //core is not available locally or remotely + autoCreateSystemColl(collectionName); + if (action != null) return; + } + } } // With a valid core... @@ -314,7 +301,6 @@ public class HttpSolrCall { // get or create/cache the parser for the core SolrRequestParsers parser = config.getRequestParsers(); - // Determine the handler from the url path if not set // (we might already have selected the cores handler) extractHandlerFromURLPath(parser); @@ -329,9 +315,7 @@ public class HttpSolrCall { invalidStates = checkStateVersionsAreValid(solrReq.getParams().get(CloudSolrClient.STATE_VERSION)); - if (usingAliases) { - processAliases(aliases, collectionsList); - } + addCollectionParamIfNeeded(getCollectionsList()); action = PROCESS; return; // we are done with a valid handler @@ -374,18 +358,24 @@ public class HttpSolrCall { } } - protected String lookupAliases(String collName) { - ZkStateReader reader = cores.getZkController().getZkStateReader(); - aliases = reader.getAliases(); - if (aliases != null && aliases.collectionAliasSize() > 0) { - usingAliases = true; - String alias = aliases.getCollectionAlias(collName); - if (alias != null) { - collectionsList = StrUtils.splitSmart(alias, ",", true); - return collectionsList.get(0); - } + /** + * Resolves the parameter as a potential comma delimited list of collections, and resolves aliases too. + * One level of aliases pointing to another alias is supported. + * De-duplicates and retains the order. + * {@link #getCollectionsList()} + */ + protected List resolveCollectionListOrAlias(String collectionStr) { + if (collectionStr == null) { + return Collections.emptyList(); } - return null; + LinkedHashSet resultList = new LinkedHashSet<>(); + Aliases aliases = getAliases(); + List inputCollections = StrUtils.splitSmart(collectionStr, ",", true); + for (String inputCollection : inputCollections) { + List resolvedCollections = aliases.resolveAliases(inputCollection); + resultList.addAll(resolvedCollections); + } + return new ArrayList<>(resultList); } /** @@ -432,9 +422,9 @@ public class HttpSolrCall { } } - protected void extractRemotePath(String corename, String origCorename, int idx) throws UnsupportedEncodingException, KeeperException, InterruptedException { + protected void extractRemotePath(String collectionName, String origCorename, int idx) throws UnsupportedEncodingException, KeeperException, InterruptedException { if (core == null && idx > 0) { - coreUrl = getRemotCoreUrl(corename, origCorename); + coreUrl = getRemotCoreUrl(collectionName, origCorename); // don't proxy for internal update requests invalidStates = checkStateVersionsAreValid(queryParams.get(CloudSolrClient.STATE_VERSION)); if (coreUrl != null @@ -745,40 +735,32 @@ public class HttpSolrCall { handler.handleRequest(solrReq, solrResp); } - protected void processAliases(Aliases aliases, - List collectionsList) { - String collection = solrReq.getParams().get(COLLECTION_PROP); - if (collection != null) { - collectionsList = StrUtils.splitSmart(collection, ",", true); + /** + * Sets the "collection" parameter on the request to the list of alias-resolved collections for this request. + * It can be avoided sometimes. + * Note: {@link org.apache.solr.handler.component.HttpShardHandler} processes this param. + * @see #getCollectionsList() + */ + protected void addCollectionParamIfNeeded(List collections) { + if (collections.isEmpty()) { + return; } - if (collectionsList != null) { - Set newCollectionsList = new HashSet<>( - collectionsList.size()); - for (String col : collectionsList) { - String al = aliases.getCollectionAlias(col); - if (al != null) { - List aliasList = StrUtils.splitSmart(al, ",", true); - newCollectionsList.addAll(aliasList); - } else { - newCollectionsList.add(col); - } - } - if (newCollectionsList.size() > 0) { - StringBuilder collectionString = new StringBuilder(); - Iterator it = newCollectionsList.iterator(); - int sz = newCollectionsList.size(); - for (int i = 0; i < sz; i++) { - collectionString.append(it.next()); - if (i < newCollectionsList.size() - 1) { - collectionString.append(","); - } - } - ModifiableSolrParams params = new ModifiableSolrParams( - solrReq.getParams()); - params.set(COLLECTION_PROP, collectionString.toString()); - solrReq.setParams(params); - } + assert cores.isZooKeeperAware(); + String collectionParam = queryParams.get(COLLECTION_PROP); + // if there is no existing collection param and the core we go to is for the expected collection, + // then we needn't add a collection param + if (collectionParam == null && // if collection param already exists, we may need to over-write it + core != null && collections.equals(Collections.singletonList(core.getCoreDescriptor().getCollectionName()))) { + return; } + String newCollectionParam = StrUtils.join(collections, ','); + if (newCollectionParam.equals(collectionParam)) { + return; + } + // TODO add a SolrRequest.getModifiableParams ? + ModifiableSolrParams params = new ModifiableSolrParams(solrReq.getParams()); + params.set(COLLECTION_PROP, newCollectionParam); + solrReq.setParams(params); } private void writeResponse(SolrQueryResponse solrRsp, QueryResponseWriter responseWriter, Method reqMethod) @@ -916,9 +898,6 @@ public class HttpSolrCall { return null; } - if (collectionsList == null) - collectionsList = new ArrayList<>(); - collectionsList.add(collectionName); String coreUrl = getCoreUrl(collectionName, origCorename, clusterState, slices, byCoreName, true); @@ -984,10 +963,8 @@ public class HttpSolrCall { SolrParams params = getQueryParams(); final ArrayList collectionRequests = new ArrayList<>(); - if (getCollectionsList() != null) { - for (String collection : getCollectionsList()) { - collectionRequests.add(new CollectionRequest(collection)); - } + for (String collection : getCollectionsList()) { + collectionRequests.add(new CollectionRequest(collection)); } // Extract collection name from the params in case of a Collection Admin request @@ -999,15 +976,7 @@ public class HttpSolrCall { else if (params.get(COLLECTION_PROP) != null) collectionRequests.add(new CollectionRequest(params.get(COLLECTION_PROP))); } - - // Handle the case when it's a /select request and collections are specified as a param - if(resource.equals("/select") && params.get("collection") != null) { - collectionRequests.clear(); - for(String collection:params.get("collection").split(",")) { - collectionRequests.add(new CollectionRequest(collection)); - } - } - + // Populate the request type if the request is select or update if(requestType == RequestType.UNKNOWN) { if(resource.startsWith("/select") || resource.startsWith("/get")) @@ -1016,15 +985,6 @@ public class HttpSolrCall { requestType = RequestType.WRITE; } - // There's no collection explicitly mentioned, let's try and extract it from the core if one exists for - // the purpose of processing this request. - if (getCore() != null && (getCollectionsList() == null || getCollectionsList().size() == 0)) { - collectionRequests.add(new CollectionRequest(getCore().getCoreDescriptor().getCollectionName())); - } - - if (getQueryParams().get(COLLECTION_PROP) != null) - collectionRequests.add(new CollectionRequest(getQueryParams().get(COLLECTION_PROP))); - return new AuthorizationContext() { @Override public SolrParams getParams() { diff --git a/solr/core/src/test/org/apache/solr/cloud/AliasIntegrationTest.java b/solr/core/src/test/org/apache/solr/cloud/AliasIntegrationTest.java index 869650df121..dad26409654 100644 --- a/solr/core/src/test/org/apache/solr/cloud/AliasIntegrationTest.java +++ b/solr/core/src/test/org/apache/solr/cloud/AliasIntegrationTest.java @@ -16,7 +16,11 @@ */ package org.apache.solr.cloud; +import java.io.IOException; +import java.util.function.Consumer; + import org.apache.solr.client.solrj.SolrQuery; +import org.apache.solr.client.solrj.SolrServerException; import org.apache.solr.client.solrj.embedded.JettySolrRunner; import org.apache.solr.client.solrj.impl.CloudSolrClient; import org.apache.solr.client.solrj.impl.HttpSolrClient; @@ -24,6 +28,8 @@ import org.apache.solr.client.solrj.request.CollectionAdminRequest; import org.apache.solr.client.solrj.request.UpdateRequest; import org.apache.solr.client.solrj.response.QueryResponse; import org.apache.solr.common.SolrException; +import org.apache.solr.common.params.ModifiableSolrParams; +import org.apache.solr.common.params.SolrParams; import org.junit.BeforeClass; import org.junit.Test; @@ -62,92 +68,50 @@ public class AliasIntegrationTest extends SolrCloudTestCase { new CollectionAdminRequest.ListAliases().process(cluster.getSolrClient()).getAliases().get("testalias")); // search for alias - QueryResponse res = cluster.getSolrClient().query("testalias", new SolrQuery("*:*")); - assertEquals(3, res.getResults().getNumFound()); - - // search for alias with random non cloud client - JettySolrRunner jetty = cluster.getRandomJetty(random()); - try (HttpSolrClient client = getHttpSolrClient(jetty.getBaseUrl().toString() + "/testalias")) { - res = client.query(new SolrQuery("*:*")); - assertEquals(3, res.getResults().getNumFound()); - } + searchSeveralWays("testalias", new SolrQuery("*:*"), 3); + + // Use a comma delimited list, one of which is an alias + searchSeveralWays("testalias,collection2", new SolrQuery("*:*"), 5); // create alias, collection2 first because it's not on every node CollectionAdminRequest.createAlias("testalias", "collection2,collection1").process(cluster.getSolrClient()); - - // search with new cloud client - try (CloudSolrClient cloudSolrClient = getCloudSolrClient(cluster.getZkServer().getZkAddress(), random().nextBoolean())) { - cloudSolrClient.setParallelUpdates(random().nextBoolean()); - SolrQuery query = new SolrQuery("*:*"); - query.set("collection", "testalias"); - res = cloudSolrClient.query(query); - assertEquals(5, res.getResults().getNumFound()); - // Try with setDefaultCollection - query = new SolrQuery("*:*"); - cloudSolrClient.setDefaultCollection("testalias"); - res = cloudSolrClient.query(query); - assertEquals(5, res.getResults().getNumFound()); - } - - // search for alias with random non cloud client - jetty = cluster.getRandomJetty(random()); - try (HttpSolrClient client = getHttpSolrClient(jetty.getBaseUrl().toString() + "/testalias")) { - SolrQuery query = new SolrQuery("*:*"); - query.set("collection", "testalias"); - res = client.query(query); - assertEquals(5, res.getResults().getNumFound()); - - // now without collections param - query = new SolrQuery("*:*"); - res = client.query(query); - assertEquals(5, res.getResults().getNumFound()); - } + searchSeveralWays("testalias", new SolrQuery("*:*"), 5); // update alias CollectionAdminRequest.createAlias("testalias", "collection2").process(cluster.getSolrClient()); - // search for alias - SolrQuery query = new SolrQuery("*:*"); - query.set("collection", "testalias"); - res = cluster.getSolrClient().query(query); - assertEquals(2, res.getResults().getNumFound()); - + searchSeveralWays("testalias", new SolrQuery("*:*"), 2); + // set alias to two collections CollectionAdminRequest.createAlias("testalias", "collection1,collection2").process(cluster.getSolrClient()); + searchSeveralWays("testalias", new SolrQuery("*:*"), 5); - query = new SolrQuery("*:*"); - query.set("collection", "testalias"); - res = cluster.getSolrClient().query(query); - assertEquals(5, res.getResults().getNumFound()); - - // try a std client - // search 1 and 2, but have no collections param - query = new SolrQuery("*:*"); - try (HttpSolrClient client = getHttpSolrClient(jetty.getBaseUrl().toString() + "/testalias")) { - res = client.query(query); - assertEquals(5, res.getResults().getNumFound()); - } + // alias pointing to alias (one level of indirection is supported; more than that is not (may or may not work) + // TODO dubious; remove? + CollectionAdminRequest.createAlias("testalias2", "testalias").process(cluster.getSolrClient()); + searchSeveralWays("testalias2", new SolrQuery("*:*"), 5); + // Test 2 aliases pointing to the same collection CollectionAdminRequest.createAlias("testalias", "collection2").process(cluster.getSolrClient()); - - // a second alias CollectionAdminRequest.createAlias("testalias2", "collection2").process(cluster.getSolrClient()); - try (HttpSolrClient client = getHttpSolrClient(jetty.getBaseUrl().toString() + "/testalias")) { - new UpdateRequest() - .add("id", "11", "a_t", "humpty dumpy4 sat on a walls") - .commit(cluster.getSolrClient(), "testalias"); - res = client.query(query); - assertEquals(3, res.getResults().getNumFound()); - } + // add one document to testalias, thus to collection2 + new UpdateRequest() + .add("id", "11", "a_t", "humpty dumpy4 sat on a walls") + .commit(cluster.getSolrClient(), "testalias"); // thus gets added to collection2 + + searchSeveralWays("testalias", new SolrQuery("*:*"), 3); CollectionAdminRequest.createAlias("testalias", "collection2,collection1").process(cluster.getSolrClient()); - - query = new SolrQuery("*:*"); - query.set("collection", "testalias"); - res = cluster.getSolrClient().query(query); - assertEquals(6, res.getResults().getNumFound()); + + searchSeveralWays("testalias", new SolrQuery("*:*"), 6); + + // add one document to testalias, which will route to collection2 because it's the first + new UpdateRequest() + .add("id", "12", "a_t", "humpty dumpy5 sat on a walls") + .commit(cluster.getSolrClient(), "testalias"); // thus gets added to collection2 + searchSeveralWays("collection2", new SolrQuery("*:*"), 4); CollectionAdminRequest.deleteAlias("testalias").process(cluster.getSolrClient()); CollectionAdminRequest.deleteAlias("testalias2").process(cluster.getSolrClient()); @@ -160,6 +124,49 @@ public class AliasIntegrationTest extends SolrCloudTestCase { assertTrue("Unexpected exception message: " + e.getMessage(), e.getMessage().contains("Collection not found: testalias")); } + private void searchSeveralWays(String collectionList, SolrParams solrQuery, int expectedNumFound) throws IOException, SolrServerException { + searchSeveralWays(collectionList, solrQuery, res -> assertEquals(expectedNumFound, res.getResults().getNumFound())); + } + + private void searchSeveralWays(String collectionList, SolrParams solrQuery, Consumer responseConsumer) throws IOException, SolrServerException { + if (random().nextBoolean()) { + // cluster's CloudSolrClient + responseConsumer.accept(cluster.getSolrClient().query(collectionList, solrQuery)); + } else { + // new CloudSolrClient (random shardLeadersOnly) + try (CloudSolrClient solrClient = getCloudSolrClient(cluster)) { + if (random().nextBoolean()) { + solrClient.setDefaultCollection(collectionList); + responseConsumer.accept(solrClient.query(null, solrQuery)); + } else { + responseConsumer.accept(solrClient.query(collectionList, solrQuery)); + } + } + } + + // HttpSolrClient + JettySolrRunner jetty = cluster.getRandomJetty(random()); + if (random().nextBoolean()) { + try (HttpSolrClient client = getHttpSolrClient(jetty.getBaseUrl().toString() + "/" + collectionList)) { + responseConsumer.accept(client.query(null, solrQuery)); + } + } else { + try (HttpSolrClient client = getHttpSolrClient(jetty.getBaseUrl().toString())) { + responseConsumer.accept(client.query(collectionList, solrQuery)); + } + } + + // Recursively do again; this time with the &collection= param + if (solrQuery.get("collection") == null) { + // put in "collection" param + ModifiableSolrParams newParams = new ModifiableSolrParams(solrQuery); + newParams.set("collection", collectionList); + String maskedColl = new String[]{null, "bogus", "collection2", "collection1"}[random().nextInt(4)]; + searchSeveralWays(maskedColl, newParams, responseConsumer); + } + + } + public void testErrorChecks() throws Exception { CollectionAdminRequest.createCollection("testErrorChecks-collection", "conf", 2, 1).process(cluster.getSolrClient()); @@ -189,6 +196,7 @@ public class AliasIntegrationTest extends SolrCloudTestCase { // Valid CollectionAdminRequest.createAlias("testalias", "testErrorChecks-collection").process(cluster.getSolrClient()); + // TODO dubious; remove? CollectionAdminRequest.createAlias("testalias2", "testalias").process(cluster.getSolrClient()); // Alias + invalid diff --git a/solr/core/src/test/org/apache/solr/cloud/DistribJoinFromCollectionTest.java b/solr/core/src/test/org/apache/solr/cloud/DistribJoinFromCollectionTest.java index 383c1ae6f1e..35ce2099df6 100644 --- a/solr/core/src/test/org/apache/solr/cloud/DistribJoinFromCollectionTest.java +++ b/solr/core/src/test/org/apache/solr/cloud/DistribJoinFromCollectionTest.java @@ -112,13 +112,13 @@ public class DistribJoinFromCollectionTest extends SolrCloudTestCase{ @Test public void testScore() throws Exception { //without score - testJoins(toColl, fromColl, toDocId, false); + testJoins(toColl, fromColl, toDocId, true); } @Test public void testNoScore() throws Exception { //with score - testJoins(toColl, fromColl, toDocId, true); + testJoins(toColl, fromColl, toDocId, false); } @@ -141,7 +141,7 @@ public class DistribJoinFromCollectionTest extends SolrCloudTestCase{ } private void testJoins(String toColl, String fromColl, String toDocId, boolean isScoresTest) - throws SolrServerException, IOException { + throws SolrServerException, IOException, InterruptedException { // verify the join with fromIndex works final String fromQ = "match_s:c^2"; CloudSolrClient client = cluster.getSolrClient(); @@ -164,8 +164,7 @@ public class DistribJoinFromCollectionTest extends SolrCloudTestCase{ // create an alias for the fromIndex and then query through the alias String alias = fromColl+"Alias"; - CollectionAdminRequest.CreateAlias request = CollectionAdminRequest.createAlias(alias,fromColl); - request.process(client); + CollectionAdminRequest.createAlias(alias, fromColl).process(client); { final String joinQ = "{!join " + anyScoreMode(isScoresTest) diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/cloud/autoscaling/DelegatingClusterStateProvider.java b/solr/solrj/src/java/org/apache/solr/client/solrj/cloud/autoscaling/DelegatingClusterStateProvider.java index 9c81005062a..e512ab39051 100644 --- a/solr/solrj/src/java/org/apache/solr/client/solrj/cloud/autoscaling/DelegatingClusterStateProvider.java +++ b/solr/solrj/src/java/org/apache/solr/client/solrj/cloud/autoscaling/DelegatingClusterStateProvider.java @@ -18,6 +18,7 @@ package org.apache.solr.client.solrj.cloud.autoscaling; import java.io.IOException; import java.util.Collections; +import java.util.List; import java.util.Map; import java.util.Set; @@ -53,20 +54,11 @@ public class DelegatingClusterStateProvider implements ClusterStateProvider { } @Override - public String getAlias(String alias) { + public List resolveAlias(String alias) { if (delegate != null) { - return delegate.getAlias(alias); + return delegate.resolveAlias(alias); } else { - return null; - } - } - - @Override - public String getCollectionName(String name) { - if (delegate != null) { - return delegate.getCollectionName(name); - } else { - return null; + return Collections.singletonList(alias); } } diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/CloudSolrClient.java b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/CloudSolrClient.java index 6a208f48c0d..f63eedd7426 100644 --- a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/CloudSolrClient.java +++ b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/CloudSolrClient.java @@ -27,6 +27,7 @@ import java.util.Collections; import java.util.HashMap; import java.util.HashSet; import java.util.Iterator; +import java.util.LinkedHashSet; import java.util.List; import java.util.Map; import java.util.Random; @@ -84,8 +85,8 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.slf4j.MDC; -import static org.apache.solr.common.params.CommonParams.ID; import static org.apache.solr.common.params.CommonParams.ADMIN_PATHS; +import static org.apache.solr.common.params.CommonParams.ID; /** * SolrJ client class to communicate with SolrCloud. @@ -204,6 +205,7 @@ public class CloudSolrClient extends SolrClient { } protected final StateCache collectionStateCache = new StateCache(); + class ExpiringCachedDocCollection { final DocCollection cached; final long cachedAt; @@ -222,7 +224,7 @@ public class CloudSolrClient extends SolrClient { > TimeUnit.NANOSECONDS.convert(timeToLiveMs, TimeUnit.MILLISECONDS); } - boolean shoulRetry() { + boolean shouldRetry() { if (maybeStale) {// we are not sure if it is stale so check with retry time if ((retriedAt == -1 || (System.nanoTime() - retriedAt) > retryExpiryTime)) { @@ -292,7 +294,7 @@ public class CloudSolrClient extends SolrClient { } } - /**Sets the cache ttl for DocCollection Objects cached . This is only applicable for collections which are persisted outside of clusterstate.json + /** Sets the cache ttl for DocCollection Objects cached . This is only applicable for collections which are persisted outside of clusterstate.json * @param seconds ttl value in seconds */ public void setCollectionCacheTTl(int seconds){ @@ -455,7 +457,7 @@ public class CloudSolrClient extends SolrClient { private NamedList directUpdate(AbstractUpdateRequest request, String collection) throws SolrServerException { UpdateRequest updateRequest = (UpdateRequest) request; - ModifiableSolrParams params = (ModifiableSolrParams) request.getParams(); + SolrParams params = request.getParams(); ModifiableSolrParams routableParams = new ModifiableSolrParams(); ModifiableSolrParams nonRoutableParams = new ModifiableSolrParams(); @@ -471,9 +473,9 @@ public class CloudSolrClient extends SolrClient { throw new SolrServerException("No collection param specified on request and no default collection has been set."); } - //Check to see if the collection is an alias. - collection = stateProvider.getCollectionName(collection); + List aliasedCollections = stateProvider.resolveAlias(collection); + collection = aliasedCollections.get(0); // pick 1st (consistent with HttpSolrCall behavior) DocCollection col = getDocCollection(collection, null); @@ -786,11 +788,16 @@ public class CloudSolrClient extends SolrClient { @Override public NamedList request(SolrRequest request, String collection) throws SolrServerException, IOException { - if (collection == null) { - collection = request.getCollection(); - if (collection == null) collection = defaultCollection; + // the collection parameter of the request overrides that of the parameter to this method + String requestCollection = request.getCollection(); + if (requestCollection != null) { + collection = requestCollection; + } else if (collection == null) { + collection = defaultCollection; } - return requestWithRetryOnStaleState(request, 0, collection); + List inputCollections = + collection == null ? Collections.emptyList() : StrUtils.splitSmart(collection, ",", true); + return requestWithRetryOnStaleState(request, 0, inputCollections); } /** @@ -798,10 +805,8 @@ public class CloudSolrClient extends SolrClient { * there's a chance that the request will fail due to cached stale state, * which means the state must be refreshed from ZK and retried. */ - protected NamedList requestWithRetryOnStaleState(SolrRequest request, int retryCount, String collection) + protected NamedList requestWithRetryOnStaleState(SolrRequest request, int retryCount, List inputCollections) throws SolrServerException, IOException { - SolrRequest originalRequest = request; - connect(); // important to call this before you start working with the ZkStateReader // build up a _stateVer_ param to pass to the server containing all of the @@ -818,8 +823,8 @@ public class CloudSolrClient extends SolrClient { isCollectionRequestOfV2 = ((V2Request) request).isPerCollectionRequest(); } boolean isAdmin = ADMIN_PATHS.contains(request.getPath()); - if (collection != null && !isAdmin && !isCollectionRequestOfV2) { // don't do _stateVer_ checking for admin, v2 api requests - Set requestedCollectionNames = getCollectionNames(collection); + if (!inputCollections.isEmpty() && !isAdmin && !isCollectionRequestOfV2) { // don't do _stateVer_ checking for admin, v2 api requests + Set requestedCollectionNames = resolveAliases(inputCollections); StringBuilder stateVerParamBuilder = null; for (String requestedCollection : requestedCollectionNames) { @@ -859,7 +864,7 @@ public class CloudSolrClient extends SolrClient { NamedList resp = null; try { - resp = sendRequest(request, collection); + resp = sendRequest(request, inputCollections); //to avoid an O(n) operation we always add STATE_VERSION to the last and try to read it from there Object o = resp == null || resp.size() == 0 ? null : resp.get(STATE_VERSION, resp.size() - 1); if(o != null && o instanceof Map) { @@ -878,7 +883,7 @@ public class CloudSolrClient extends SolrClient { // don't do retry support for admin requests // or if the request doesn't have a collection specified // or request is v2 api and its method is not GET - if (collection == null || isAdmin || (request instanceof V2Request && request.getMethod() != SolrRequest.METHOD.GET)) { + if (inputCollections.isEmpty() || isAdmin || (request instanceof V2Request && request.getMethod() != SolrRequest.METHOD.GET)) { if (exc instanceof SolrServerException) { throw (SolrServerException)exc; } else if (exc instanceof IOException) { @@ -894,8 +899,8 @@ public class CloudSolrClient extends SolrClient { int errorCode = (rootCause instanceof SolrException) ? ((SolrException)rootCause).code() : SolrException.ErrorCode.UNKNOWN.code; - log.error("Request to collection {} failed due to ("+errorCode+ - ") {}, retry? "+retryCount, collection, rootCause.toString()); + log.error("Request to collection {} failed due to (" + errorCode + ") {}, retry? " + retryCount, + inputCollections, rootCause.toString()); boolean wasCommError = (rootCause instanceof ConnectException || @@ -907,7 +912,7 @@ public class CloudSolrClient extends SolrClient { // it was a communication error. it is likely that // the node to which the request to be sent is down . So , expire the state // so that the next attempt would fetch the fresh state - // just re-read state for all of them, if it has not been retired + // just re-read state for all of them, if it has not been retried // in retryExpiryTime time for (DocCollection ext : requestedCollections) { ExpiringCachedDocCollection cacheEntry = collectionStateCache.get(ext.getName()); @@ -919,7 +924,7 @@ public class CloudSolrClient extends SolrClient { // and we could not get any information from the server //it is probably not worth trying again and again because // the state would not have been updated - return requestWithRetryOnStaleState(request, retryCount + 1, collection); + return requestWithRetryOnStaleState(request, retryCount + 1, inputCollections); } } @@ -963,8 +968,8 @@ public class CloudSolrClient extends SolrClient { // if the state was stale, then we retry the request once with new state pulled from Zk if (stateWasStale) { - log.warn("Re-trying request to collection(s) "+collection+" after stale state error from server."); - resp = requestWithRetryOnStaleState(request, retryCount+1, collection); + log.warn("Re-trying request to collection(s) "+inputCollections+" after stale state error from server."); + resp = requestWithRetryOnStaleState(request, retryCount+1, inputCollections); } else { if(exc instanceof SolrException) { throw exc; @@ -981,137 +986,97 @@ public class CloudSolrClient extends SolrClient { return resp; } - protected NamedList sendRequest(SolrRequest request, String collection) + protected NamedList sendRequest(SolrRequest request, List inputCollections) throws SolrServerException, IOException { connect(); boolean sendToLeaders = false; - List replicas = null; - + if (request instanceof IsUpdateRequest) { if (request instanceof UpdateRequest) { + String collection = inputCollections.isEmpty() ? null : inputCollections.get(0); // getting first mimics HttpSolrCall NamedList response = directUpdate((AbstractUpdateRequest) request, collection); if (response != null) { return response; } } sendToLeaders = true; - replicas = new ArrayList<>(); } SolrParams reqParams = request.getParams(); - if (reqParams == null) { + if (reqParams == null) { // TODO fix getParams to never return null! reqParams = new ModifiableSolrParams(); } - List theUrlList = new ArrayList<>(); + + final Set liveNodes = stateProvider.getLiveNodes(); + + final List theUrlList = new ArrayList<>(); // we populate this as follows... + if (request instanceof V2Request) { - Set liveNodes = stateProvider.getLiveNodes(); if (!liveNodes.isEmpty()) { List liveNodesList = new ArrayList<>(liveNodes); Collections.shuffle(liveNodesList, rand); theUrlList.add(ZkStateReader.getBaseUrlForNodeName(liveNodesList.get(0), (String) stateProvider.getClusterProperty(ZkStateReader.URL_SCHEME,"http"))); } + } else if (ADMIN_PATHS.contains(request.getPath())) { - Set liveNodes = stateProvider.getLiveNodes(); for (String liveNode : liveNodes) { theUrlList.add(ZkStateReader.getBaseUrlForNodeName(liveNode, (String) stateProvider.getClusterProperty(ZkStateReader.URL_SCHEME,"http"))); } - } else { - - if (collection == null) { - throw new SolrServerException( - "No collection param specified on request and no default collection has been set."); - } - Set collectionNames = getCollectionNames(collection); - if (collectionNames.size() == 0) { + } else { // Typical... + Set collectionNames = resolveAliases(inputCollections); + if (collectionNames.isEmpty()) { throw new SolrException(ErrorCode.BAD_REQUEST, - "Could not find collection: " + collection); + "No collection param specified on request and no default collection has been set: " + inputCollections); } - String shardKeys = reqParams.get(ShardParams._ROUTE_); - // TODO: not a big deal because of the caching, but we could avoid looking - // at every shard - // when getting leaders if we tweaked some things + // at every shard when getting leaders if we tweaked some things - // Retrieve slices from the cloud state and, for each collection - // specified, - // add it to the Map of slices. + // Retrieve slices from the cloud state and, for each collection specified, add it to the Map of slices. Map slices = new HashMap<>(); + String shardKeys = reqParams.get(ShardParams._ROUTE_); for (String collectionName : collectionNames) { DocCollection col = getDocCollection(collectionName, null); Collection routeSlices = col.getRouter().getSearchSlices(shardKeys, reqParams , col); ClientUtils.addSlices(slices, collectionName, routeSlices, true); } - Set liveNodes = stateProvider.getLiveNodes(); - List leaderUrlList = null; - List urlList = null; - List replicasList = null; - - // build a map of unique nodes + // Gather URLs, grouped by leader or replica // TODO: allow filtering by group, role, etc - Map nodes = new HashMap<>(); - List urlList2 = new ArrayList<>(); + Set seenNodes = new HashSet<>(); + List replicas = new ArrayList<>(); + String joinedInputCollections = StrUtils.join(inputCollections, ','); for (Slice slice : slices.values()) { for (ZkNodeProps nodeProps : slice.getReplicasMap().values()) { ZkCoreNodeProps coreNodeProps = new ZkCoreNodeProps(nodeProps); String node = coreNodeProps.getNodeName(); - if (!liveNodes.contains(coreNodeProps.getNodeName()) - || Replica.State.getState(coreNodeProps.getState()) != Replica.State.ACTIVE) continue; - if (nodes.put(node, nodeProps) == null) { - if (!sendToLeaders || coreNodeProps.isLeader()) { - String url; - if (reqParams.get(UpdateParams.COLLECTION) == null) { - url = ZkCoreNodeProps.getCoreUrl(nodeProps.getStr(ZkStateReader.BASE_URL_PROP), collection); - } else { - url = coreNodeProps.getCoreUrl(); - } - urlList2.add(url); + if (!liveNodes.contains(node) // Must be a live node to continue + || Replica.State.getState(coreNodeProps.getState()) != Replica.State.ACTIVE) // Must be an ACTIVE replica to continue + continue; + if (seenNodes.add(node)) { // if we haven't yet collected a URL to this node... + String url = ZkCoreNodeProps.getCoreUrl(nodeProps.getStr(ZkStateReader.BASE_URL_PROP), joinedInputCollections); + if (sendToLeaders && coreNodeProps.isLeader()) { + theUrlList.add(url); // put leaders here eagerly (if sendToLeader mode) } else { - String url; - if (reqParams.get(UpdateParams.COLLECTION) == null) { - url = ZkCoreNodeProps.getCoreUrl(nodeProps.getStr(ZkStateReader.BASE_URL_PROP), collection); - } else { - url = coreNodeProps.getCoreUrl(); - } - replicas.add(url); + replicas.add(url); // replicas here } } } } - - if (sendToLeaders) { - leaderUrlList = urlList2; - replicasList = replicas; - } else { - urlList = urlList2; - } - - if (sendToLeaders) { - theUrlList = new ArrayList<>(leaderUrlList.size()); - theUrlList.addAll(leaderUrlList); - } else { - theUrlList = new ArrayList<>(urlList.size()); - theUrlList.addAll(urlList); - } + // Shuffle the leaders, if any (none if !sendToLeaders) Collections.shuffle(theUrlList, rand); - if (sendToLeaders) { - ArrayList theReplicas = new ArrayList<>( - replicasList.size()); - theReplicas.addAll(replicasList); - Collections.shuffle(theReplicas, rand); - theUrlList.addAll(theReplicas); - } - + + // Shuffle the replicas, if any, and append to our list + Collections.shuffle(replicas, rand); + theUrlList.addAll(replicas); + if (theUrlList.isEmpty()) { - for (String s : collectionNames) { - if (s != null) collectionStateCache.remove(s); - } + collectionStateCache.keySet().removeAll(collectionNames); throw new SolrException(SolrException.ErrorCode.INVALID_STATE, "Could not find a healthy node to handle the request."); } @@ -1122,24 +1087,20 @@ public class CloudSolrClient extends SolrClient { return rsp.getResponse(); } - private Set getCollectionNames(String collection) { - // Extract each comma separated collection name and store in a List. - List rawCollectionsList = StrUtils.splitSmart(collection, ",", true); - Set collectionNames = new HashSet<>(); - // validate collections - for (String collectionName : rawCollectionsList) { + /** Resolves the input collections to their possible aliased collections. Doesn't validate collection existence. */ + private LinkedHashSet resolveAliases(List inputCollections) { + LinkedHashSet collectionNames = new LinkedHashSet<>(); // consistent ordering + for (String collectionName : inputCollections) { if (stateProvider.getState(collectionName) == null) { - String alias = stateProvider.getAlias(collectionName); - if (alias != null) { - List aliasList = StrUtils.splitSmart(alias, ",", true); - collectionNames.addAll(aliasList); - continue; + // perhaps it's an alias + List aliasedCollections = stateProvider.resolveAlias(collectionName); + // one more level of alias indirection... (dubious that we should support this) + for (String aliasedCollection : aliasedCollections) { + collectionNames.addAll(stateProvider.resolveAlias(aliasedCollection)); } - - throw new SolrException(ErrorCode.BAD_REQUEST, "Collection not found: " + collectionName); - } - - collectionNames.add(collectionName); + } else { + collectionNames.add(collectionName); // it's a collection + } } return collectionNames; } @@ -1199,7 +1160,7 @@ public class CloudSolrClient extends SolrClient { DocCollection col = cacheEntry == null ? null : cacheEntry.cached; if (col != null) { if (expectedVersion <= col.getZNodeVersion() - && !cacheEntry.shoulRetry()) return col; + && !cacheEntry.shouldRetry()) return col; } CollectionRef ref = getCollectionRef(collection); @@ -1220,7 +1181,7 @@ public class CloudSolrClient extends SolrClient { col = cacheEntry == null ? null : cacheEntry.cached; if (col != null) { if (expectedVersion <= col.getZNodeVersion() - && !cacheEntry.shoulRetry()) return col; + && !cacheEntry.shouldRetry()) return col; } // We are going to fetch a new version // we MUST try to get a new version @@ -1466,7 +1427,7 @@ public class CloudSolrClient extends SolrClient { } /** - * Tells {@link Builder} that created clients should send updats only to shard leaders. + * Tells {@link Builder} that created clients should send updates only to shard leaders. */ public Builder sendUpdatesOnlyToShardLeaders() { shardLeadersOnly = true; diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/ClusterStateProvider.java b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/ClusterStateProvider.java index 7cb5d514302..c28545257bd 100644 --- a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/ClusterStateProvider.java +++ b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/ClusterStateProvider.java @@ -19,6 +19,7 @@ package org.apache.solr.client.solrj.impl; import java.io.Closeable; import java.io.IOException; import java.util.Map; +import java.util.List; import java.util.Set; import org.apache.solr.common.cloud.ClusterState; @@ -37,15 +38,10 @@ public interface ClusterStateProvider extends Closeable { Set getLiveNodes(); /** - * Given an alias, returns the collection name that this alias points to + * Given a collection alias, returns a list of collections it points to, or returns a singleton list of the input if + * it's not an alias. */ - String getAlias(String alias); - - /** - * Given a name, returns the collection name if an alias by that name exists, or - * returns the name itself, if no alias exists. - */ - String getCollectionName(String name); + List resolveAlias(String alias); /** * Obtain the current cluster state. diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/HttpClusterStateProvider.java b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/HttpClusterStateProvider.java index 54f3faecc5c..d617d165f65 100644 --- a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/HttpClusterStateProvider.java +++ b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/HttpClusterStateProvider.java @@ -32,6 +32,7 @@ import org.apache.solr.client.solrj.SolrServerException; import org.apache.solr.client.solrj.impl.HttpSolrClient.RemoteSolrException; import org.apache.solr.client.solrj.request.CollectionAdminRequest; import org.apache.solr.client.solrj.request.QueryRequest; +import org.apache.solr.common.cloud.Aliases; import org.apache.solr.common.cloud.ClusterState; import org.apache.solr.common.cloud.ClusterState.CollectionRef; import org.apache.solr.common.cloud.ZkStateReader; @@ -47,7 +48,7 @@ public class HttpClusterStateProvider implements ClusterStateProvider { private String urlScheme; volatile Set liveNodes; long liveNodesTimestamp = 0; - volatile Map aliases; + volatile Map> aliases; long aliasesTimestamp = 0; private int cacheTimeout = 5; // the liveNodes and aliases cache will be invalidated after 5 secs @@ -191,12 +192,11 @@ public class HttpClusterStateProvider implements ClusterStateProvider { } @Override - public String getAlias(String alias) { - Map aliases = getAliases(false); - return aliases.get(alias); + public List resolveAlias(String aliasName) { + return Aliases.resolveAliasesGivenAliasMap(getAliases(false), aliasName); } - private Map getAliases(boolean forceFetch) { + private Map> getAliases(boolean forceFetch) { if (this.liveNodes == null) { throw new RuntimeException("We don't know of any live_nodes to fetch the" + " latest aliases information from. " @@ -212,10 +212,10 @@ public class HttpClusterStateProvider implements ClusterStateProvider { withBaseSolrUrl(ZkStateReader.getBaseUrlForNodeName(nodeName, urlScheme)). withHttpClient(httpClient).build()) { - Map aliases = new CollectionAdminRequest.ListAliases().process(client).getAliases(); + Map> aliases = new CollectionAdminRequest.ListAliases().process(client).getAliasesAsLists(); this.aliases = aliases; this.aliasesTimestamp = System.nanoTime(); - return Collections.unmodifiableMap(aliases); + return Collections.unmodifiableMap(this.aliases); } catch (SolrServerException | RemoteSolrException | IOException e) { // Situation where we're hitting an older Solr which doesn't have LISTALIASES if (e instanceof RemoteSolrException && ((RemoteSolrException)e).code()==400) { @@ -240,12 +240,6 @@ public class HttpClusterStateProvider implements ClusterStateProvider { } } - @Override - public String getCollectionName(String name) { - Map aliases = getAliases(false); - return aliases.containsKey(name) ? aliases.get(name): name; - } - @Override public ClusterState getClusterState() throws IOException { for (String nodeName: liveNodes) { diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/ZkClientClusterStateProvider.java b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/ZkClientClusterStateProvider.java index 66f71437ebf..968e5141cc9 100644 --- a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/ZkClientClusterStateProvider.java +++ b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/ZkClientClusterStateProvider.java @@ -22,11 +22,11 @@ import java.lang.invoke.MethodHandles; import java.nio.file.Path; import java.util.Collection; import java.util.Collections; +import java.util.List; import java.util.Map; import java.util.Set; import org.apache.solr.common.SolrException; -import org.apache.solr.common.cloud.Aliases; import org.apache.solr.common.cloud.ClusterState; import org.apache.solr.common.cloud.ZkStateReader; import org.apache.solr.common.cloud.ZooKeeperException; @@ -83,9 +83,8 @@ public class ZkClientClusterStateProvider implements ClusterStateProvider { @Override - public String getAlias(String alias) { - Aliases aliases = zkStateReader.getAliases(); - return aliases.getCollectionAlias(alias); + public List resolveAlias(String alias) { + return zkStateReader.getAliases().resolveAliases(alias); // if not an alias, returns itself } @Override @@ -103,18 +102,6 @@ public class ZkClientClusterStateProvider implements ClusterStateProvider { return def; } - @Override - public String getCollectionName(String name) { - Aliases aliases = zkStateReader.getAliases(); - if (aliases != null) { - Map collectionAliases = aliases.getCollectionAliasMap(); - if (collectionAliases != null && collectionAliases.containsKey(name)) { - name = collectionAliases.get(name); - } - } - return name; - } - @Override public ClusterState getClusterState() throws IOException { return zkStateReader.getClusterState(); diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/io/stream/CloudSolrStream.java b/solr/solrj/src/java/org/apache/solr/client/solrj/io/stream/CloudSolrStream.java index d63d33d3302..32cf15e297a 100644 --- a/solr/solrj/src/java/org/apache/solr/client/solrj/io/stream/CloudSolrStream.java +++ b/solr/solrj/src/java/org/apache/solr/client/solrj/io/stream/CloudSolrStream.java @@ -26,6 +26,7 @@ import java.util.List; import java.util.Locale; import java.util.Map; import java.util.Map.Entry; +import java.util.Objects; import java.util.TreeSet; import java.util.concurrent.Callable; import java.util.concurrent.ExecutorService; @@ -46,7 +47,6 @@ import org.apache.solr.client.solrj.io.stream.expr.StreamExpression; import org.apache.solr.client.solrj.io.stream.expr.StreamExpressionNamedParameter; import org.apache.solr.client.solrj.io.stream.expr.StreamExpressionValue; import org.apache.solr.client.solrj.io.stream.expr.StreamFactory; -import org.apache.solr.common.cloud.Aliases; import org.apache.solr.common.cloud.ClusterState; import org.apache.solr.common.cloud.DocCollection; import org.apache.solr.common.cloud.Slice; @@ -55,7 +55,6 @@ import org.apache.solr.common.params.ModifiableSolrParams; import org.apache.solr.common.params.SolrParams; import org.apache.solr.common.util.ExecutorUtil; import org.apache.solr.common.util.SolrjNamedThreadFactory; -import org.apache.solr.common.util.StrUtils; import static org.apache.solr.common.params.CommonParams.DISTRIB; import static org.apache.solr.common.params.CommonParams.SORT; @@ -331,9 +330,21 @@ public class CloudSolrStream extends TupleStream implements Expressible { Map collectionsMap = clusterState.getCollectionsMap(); - // Check collection case sensitive - if(collectionsMap.containsKey(collectionName)) { - return collectionsMap.get(collectionName).getActiveSlices(); + //TODO we should probably split collection by comma to query more than one + // which is something already supported in other parts of Solr + + // check for alias or collection + List collections = checkAlias + ? zkStateReader.getAliases().resolveAliases(collectionName) // if not an alias, returns collectionName + : Collections.singletonList(collectionName); + // Lookup all actives slices for these collections + List slices = collections.stream() + .map(collectionsMap::get) + .filter(Objects::nonNull) + .flatMap(docCol -> docCol.getActiveSlices().stream()) + .collect(Collectors.toList()); + if (!slices.isEmpty()) { + return slices; } // Check collection case insensitive @@ -343,23 +354,6 @@ public class CloudSolrStream extends TupleStream implements Expressible { } } - if(checkAlias) { - // check for collection alias - Aliases aliases = zkStateReader.getAliases(); - String alias = aliases.getCollectionAlias(collectionName); - if (alias != null) { - Collection slices = new ArrayList<>(); - - List aliasList = StrUtils.splitSmart(alias, ",", true); - for (String aliasCollectionName : aliasList) { - // Add all active slices for this alias collection - slices.addAll(collectionsMap.get(aliasCollectionName).getActiveSlices()); - } - - return slices; - } - } - throw new IOException("Slices not found for " + collectionName); } diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/io/stream/TupleStream.java b/solr/solrj/src/java/org/apache/solr/client/solrj/io/stream/TupleStream.java index 5a66f323d3f..e178040812f 100644 --- a/solr/solrj/src/java/org/apache/solr/client/solrj/io/stream/TupleStream.java +++ b/solr/solrj/src/java/org/apache/solr/client/solrj/io/stream/TupleStream.java @@ -37,14 +37,11 @@ import org.apache.solr.client.solrj.io.stream.expr.Explanation; import org.apache.solr.client.solrj.io.stream.expr.StreamFactory; import org.apache.solr.common.IteratorWriter; import org.apache.solr.common.MapWriter; -import org.apache.solr.common.cloud.Aliases; import org.apache.solr.common.cloud.ClusterState; -import org.apache.solr.common.cloud.DocCollection; import org.apache.solr.common.cloud.Replica; import org.apache.solr.common.cloud.Slice; import org.apache.solr.common.cloud.ZkCoreNodeProps; import org.apache.solr.common.cloud.ZkStateReader; -import org.apache.solr.common.util.StrUtils; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -141,7 +138,7 @@ public abstract class TupleStream implements Closeable, Serializable, MapWriter CloudSolrClient cloudSolrClient = streamContext.getSolrClientCache().getCloudSolrClient(zkHost); ZkStateReader zkStateReader = cloudSolrClient.getZkStateReader(); ClusterState clusterState = zkStateReader.getClusterState(); - Collection slices = getSlices(collection, zkStateReader, true); + Collection slices = CloudSolrStream.getSlices(collection, zkStateReader, true); Set liveNodes = clusterState.getLiveNodes(); for(Slice slice : slices) { Collection replicas = slice.getReplicas(); @@ -162,45 +159,6 @@ public abstract class TupleStream implements Closeable, Serializable, MapWriter return shards; } - public static Collection getSlices(String collectionName, - ZkStateReader zkStateReader, - boolean checkAlias) throws IOException { - ClusterState clusterState = zkStateReader.getClusterState(); - - Map collectionsMap = clusterState.getCollectionsMap(); - - // Check collection case sensitive - if(collectionsMap.containsKey(collectionName)) { - return collectionsMap.get(collectionName).getActiveSlices(); - } - - // Check collection case insensitive - for(String collectionMapKey : collectionsMap.keySet()) { - if(collectionMapKey.equalsIgnoreCase(collectionName)) { - return collectionsMap.get(collectionMapKey).getActiveSlices(); - } - } - - if(checkAlias) { - // check for collection alias - Aliases aliases = zkStateReader.getAliases(); - String alias = aliases.getCollectionAlias(collectionName); - if (alias != null) { - Collection slices = new ArrayList<>(); - - List aliasList = StrUtils.splitSmart(alias, ",", true); - for (String aliasCollectionName : aliasList) { - // Add all active slices for this alias collection - slices.addAll(collectionsMap.get(aliasCollectionName).getActiveSlices()); - } - - return slices; - } - } - - throw new IOException("Slices not found for " + collectionName); - } - public static class IgnoreException extends IOException { public void printStackTrace(PrintWriter pw) { pw.print("Early Client Disconnect"); diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/response/CollectionAdminResponse.java b/solr/solrj/src/java/org/apache/solr/client/solrj/response/CollectionAdminResponse.java index c50ef37bfce..e36f8d95354 100644 --- a/solr/solrj/src/java/org/apache/solr/client/solrj/response/CollectionAdminResponse.java +++ b/solr/solrj/src/java/org/apache/solr/client/solrj/response/CollectionAdminResponse.java @@ -18,8 +18,10 @@ package org.apache.solr.client.solrj.response; import java.util.Collections; import java.util.HashMap; +import java.util.List; import java.util.Map; +import org.apache.solr.common.cloud.Aliases; import org.apache.solr.common.util.NamedList; public class CollectionAdminResponse extends SolrResponseBase @@ -76,6 +78,11 @@ public class CollectionAdminResponse extends SolrResponseBase return Collections.emptyMap(); } + public Map> getAliasesAsLists() { + // TODO we compute on each call... should this be done once & cached? + return Aliases.convertMapOfCommaDelimitedToMapOfList(getAliases()); + } + @SuppressWarnings("unchecked") public Map> getCollectionNodesStatus() { diff --git a/solr/solrj/src/java/org/apache/solr/common/cloud/Aliases.java b/solr/solrj/src/java/org/apache/solr/common/cloud/Aliases.java index e927ddb554b..4ff748cdbda 100644 --- a/solr/solrj/src/java/org/apache/solr/common/cloud/Aliases.java +++ b/solr/solrj/src/java/org/apache/solr/common/cloud/Aliases.java @@ -16,47 +16,132 @@ */ package org.apache.solr.common.cloud; +import java.util.ArrayList; import java.util.Collections; import java.util.HashMap; +import java.util.LinkedHashMap; +import java.util.List; import java.util.Map; +import org.apache.solr.common.util.StrUtils; +import org.apache.solr.common.util.Utils; + +/** + * Holds collection aliases -- virtual collections that point to one or more other collections. + * We might add other types of aliases here some day. + * Immutable. + */ public class Aliases { - private Map> aliasMap; + public static final Aliases EMPTY = new Aliases(Collections.emptyMap()); - public Aliases(Map> aliasMap) { + /** Map of "collection" string constant to -> + * alias name -> comma delimited list of collections */ + private final Map> aliasMap; // not-null + + private final Map> collectionAliasListMap; // not-null; computed from aliasMap + + public static Aliases fromJSON(byte[] bytes) { + if (bytes == null || bytes.length == 0) { + return EMPTY; + } + return new Aliases((Map>) Utils.fromJSON(bytes)); + } + + private Aliases(Map> aliasMap) { this.aliasMap = aliasMap; + collectionAliasListMap = convertMapOfCommaDelimitedToMapOfList(getCollectionAliasMap()); } - public Aliases() { - this.aliasMap = new HashMap<>(); + public static Map> convertMapOfCommaDelimitedToMapOfList(Map collectionAliasMap) { + Map> collectionAliasListMap = new LinkedHashMap<>(collectionAliasMap.size()); + for (Map.Entry entry : collectionAliasMap.entrySet()) { + collectionAliasListMap.put(entry.getKey(), StrUtils.splitSmart(entry.getValue(), ",", true)); + } + return collectionAliasListMap; } - + + /** + * Returns an unmodifiable Map of collection aliases mapped to a comma delimited list of what the alias maps to. + * Does not return null. + * Prefer use of {@link #getCollectionAliasListMap()} instead, where appropriate. + */ public Map getCollectionAliasMap() { Map cam = aliasMap.get("collection"); - if (cam == null) return null; - return Collections.unmodifiableMap(cam); - } - - public Map> getAliasMap() { - return Collections.unmodifiableMap(aliasMap); + return cam == null ? Collections.emptyMap() : Collections.unmodifiableMap(cam); } - public int collectionAliasSize() { - Map cam = aliasMap.get("collection"); - if (cam == null) return 0; - return cam.size(); + /** + * Returns an unmodifiable Map of collection aliases mapped to a list of what the alias maps to. + * Does not return null. + */ + public Map> getCollectionAliasListMap() { + return Collections.unmodifiableMap(collectionAliasListMap); } + public boolean hasCollectionAliases() { + return !collectionAliasListMap.isEmpty(); + } + + /** + * Returns a list of collections that the input alias name maps to. If there + * are none, the input is returned. One level of alias indirection is supported (alias to alias to collection). + * Treat the result as unmodifiable. + */ + public List resolveAliases(String aliasName) { + return resolveAliasesGivenAliasMap(collectionAliasListMap, aliasName); + } + + /** @lucene.internal */ + public static List resolveAliasesGivenAliasMap(Map> collectionAliasListMap, String aliasName) { + //return collectionAliasListMap.getOrDefault(aliasName, Collections.singletonList(aliasName)); + // TODO deprecate and remove this dubious feature? + // Due to another level of indirection, this is more complicated... + List level1 = collectionAliasListMap.get(aliasName); + if (level1 == null) { + return Collections.singletonList(aliasName);// is a collection + } + List result = new ArrayList<>(level1.size()); + for (String level1Alias : level1) { + List level2 = collectionAliasListMap.get(level1Alias); + if (level2 == null) { + result.add(level1Alias); + } else { + result.addAll(level2); + } + } + return result; + } + + /** + * Creates a new Aliases instance with the same data as the current one but with a modification based on the + * parameters. If {@code collections} is null, then the {@code alias} is removed, otherwise it is added/updated. + */ + public Aliases cloneWithCollectionAlias(String alias, String collections) { + Map newCollectionMap = new HashMap<>(getCollectionAliasMap()); + if (collections == null) { + newCollectionMap.remove(alias); + } else { + newCollectionMap.put(alias, collections); + } + if (newCollectionMap.isEmpty()) { + return EMPTY; + } else { + return new Aliases(Collections.singletonMap("collection", newCollectionMap)); + } + } + + /** Serialize to ZooKeeper. */ + public byte[] toJSON() { + if (collectionAliasListMap.isEmpty()) { + return null; + } else { + return Utils.toJSON(aliasMap); + } + } + @Override public String toString() { return "Aliases [aliasMap=" + aliasMap + "]"; } - - public String getCollectionAlias(String collectionName) { - Map cam = aliasMap.get("collection"); - if (cam == null) return null; - return cam.get(collectionName); - } - } diff --git a/solr/solrj/src/java/org/apache/solr/common/cloud/ClusterState.java b/solr/solrj/src/java/org/apache/solr/common/cloud/ClusterState.java index 8e52e419270..bfe33c5e100 100644 --- a/solr/solrj/src/java/org/apache/solr/common/cloud/ClusterState.java +++ b/solr/solrj/src/java/org/apache/solr/common/cloud/ClusterState.java @@ -232,15 +232,6 @@ public class ClusterState implements JSONWriter.Writable { return new ClusterState( liveNodes, collections,version); } - public static Aliases load(byte[] bytes) { - if (bytes == null || bytes.length == 0) { - return new Aliases(); - } - Map> aliasMap = (Map>) Utils.fromJSON(bytes); - - return new Aliases(aliasMap); - } - // TODO move to static DocCollection.loadFromMap private static DocCollection collectionFromObjects(String name, Map objs, Integer version, String znode) { Map props; diff --git a/solr/solrj/src/java/org/apache/solr/common/cloud/ZkStateReader.java b/solr/solrj/src/java/org/apache/solr/common/cloud/ZkStateReader.java index e45565f133a..4695d7d0719 100644 --- a/solr/solrj/src/java/org/apache/solr/common/cloud/ZkStateReader.java +++ b/solr/solrj/src/java/org/apache/solr/common/cloud/ZkStateReader.java @@ -257,7 +257,7 @@ public class ZkStateReader implements Closeable { private final boolean closeClient; - private volatile Aliases aliases = new Aliases(); + private volatile Aliases aliases = Aliases.EMPTY; private volatile boolean closed = false; @@ -385,8 +385,10 @@ public class ZkStateReader implements Closeable { public void updateLiveNodes() throws KeeperException, InterruptedException { refreshLiveNodes(null); } - + + /** Never null. */ public Aliases getAliases() { + assert aliases != null; return aliases; } @@ -455,7 +457,7 @@ public class ZkStateReader implements Closeable { final Watcher thisWatch = this; final Stat stat = new Stat(); final byte[] data = zkClient.getData(ALIASES, thisWatch, stat, true); - ZkStateReader.this.aliases = ClusterState.load(data); + ZkStateReader.this.aliases = Aliases.fromJSON(data); LOG.debug("New alias definition is: " + ZkStateReader.this.aliases.toString()); } } catch (KeeperException.ConnectionLossException | KeeperException.SessionExpiredException e) { @@ -896,7 +898,7 @@ public class ZkStateReader implements Closeable { public void updateAliases() throws KeeperException, InterruptedException { final byte[] data = zkClient.getData(ALIASES, null, null, true); - this.aliases = ClusterState.load(data); + this.aliases = Aliases.fromJSON(data); } /** diff --git a/solr/solrj/src/java/org/apache/solr/common/util/StrUtils.java b/solr/solrj/src/java/org/apache/solr/common/util/StrUtils.java index a20c6e2af35..117de5747dc 100644 --- a/solr/solrj/src/java/org/apache/solr/common/util/StrUtils.java +++ b/solr/solrj/src/java/org/apache/solr/common/util/StrUtils.java @@ -88,6 +88,7 @@ public class StrUtils { * @param s the string to split * @param separator the separator to split on * @param decode decode backslash escaping + * @return not null */ public static List splitSmart(String s, String separator, boolean decode) { ArrayList lst = new ArrayList<>(2); diff --git a/solr/solrj/src/test/org/apache/solr/client/solrj/impl/CloudSolrClientCacheTest.java b/solr/solrj/src/test/org/apache/solr/client/solrj/impl/CloudSolrClientCacheTest.java index a3dd4365c16..cfcfcd6227e 100644 --- a/solr/solrj/src/test/org/apache/solr/client/solrj/impl/CloudSolrClientCacheTest.java +++ b/solr/solrj/src/test/org/apache/solr/client/solrj/impl/CloudSolrClientCacheTest.java @@ -22,6 +22,7 @@ import java.net.SocketException; import java.util.Collections; import java.util.HashMap; import java.util.HashSet; +import java.util.List; import java.util.Map; import java.util.Set; import java.util.concurrent.atomic.AtomicInteger; @@ -130,13 +131,8 @@ public class CloudSolrClientCacheTest extends SolrTestCaseJ4 { } @Override - public String getAlias(String collection) { - return collection; - } - - @Override - public String getCollectionName(String name) { - return name; + public List resolveAlias(String collection) { + return Collections.singletonList(collection); } @Override diff --git a/solr/solrj/src/test/org/apache/solr/client/solrj/io/sql/JdbcTest.java b/solr/solrj/src/test/org/apache/solr/client/solrj/io/sql/JdbcTest.java index ce14907eb9d..bb07c4502e5 100644 --- a/solr/solrj/src/test/org/apache/solr/client/solrj/io/sql/JdbcTest.java +++ b/solr/solrj/src/test/org/apache/solr/client/solrj/io/sql/JdbcTest.java @@ -28,7 +28,6 @@ import java.sql.Types; import java.util.ArrayList; import java.util.Arrays; import java.util.List; -import java.util.Map; import java.util.Properties; import java.util.Set; import java.util.SortedSet; @@ -555,13 +554,7 @@ public class JdbcTest extends SolrCloudTestCase { tables.addAll(collectionsSet); Aliases aliases = zkStateReader.getAliases(); - if(aliases != null) { - Map collectionAliasMap = aliases.getCollectionAliasMap(); - if(collectionAliasMap != null) { - Set aliasesSet = collectionAliasMap.keySet(); - tables.addAll(aliasesSet); - } - } + tables.addAll(aliases.getCollectionAliasListMap().keySet()); try(ResultSet rs = databaseMetaData.getTables(null, zkHost, "%", null)) { for(String table : tables) { diff --git a/solr/test-framework/src/java/org/apache/solr/SolrTestCaseJ4.java b/solr/test-framework/src/java/org/apache/solr/SolrTestCaseJ4.java index 8e129c7b292..9d9a9dc55bf 100644 --- a/solr/test-framework/src/java/org/apache/solr/SolrTestCaseJ4.java +++ b/solr/test-framework/src/java/org/apache/solr/SolrTestCaseJ4.java @@ -84,6 +84,7 @@ import org.apache.solr.client.solrj.impl.HttpSolrClient.Builder; import org.apache.solr.client.solrj.impl.LBHttpSolrClient; import org.apache.solr.client.solrj.util.ClientUtils; import org.apache.solr.cloud.IpTables; +import org.apache.solr.cloud.MiniSolrCloudCluster; import org.apache.solr.common.SolrDocument; import org.apache.solr.common.SolrDocumentList; import org.apache.solr.common.SolrException; @@ -2224,6 +2225,14 @@ public abstract class SolrTestCaseJ4 extends LuceneTestCase { super(); } + public CloudSolrClient.Builder withCluster(MiniSolrCloudCluster cluster) { + if (random().nextBoolean()) { + return withZkHost(cluster.getZkServer().getZkAddress()); + } else { + return withSolrUrl(cluster.getRandomJetty(random()).getBaseUrl().toString()); + } + } + @Override public CloudSolrClient.Builder sendDirectUpdatesToShardLeadersOnly() { configuredDUTflag = true; @@ -2247,7 +2256,7 @@ public abstract class SolrTestCaseJ4 extends LuceneTestCase { @Override public CloudSolrClient build() { if (configuredDUTflag == false) { - // flag value not explicity configured + // flag value not explicitly configured if (random().nextBoolean()) { // so randomly choose a value randomlyChooseDirectUpdatesToLeadersOnly(); @@ -2262,7 +2271,7 @@ public abstract class SolrTestCaseJ4 extends LuceneTestCase { /** * This method may randomize unspecified aspects of the resulting SolrClient. - * Tests that do not wish to have any randomized behavior should use the + * Tests that do not wish to have any randomized behavior should use the * {@link org.apache.solr.client.solrj.impl.CloudSolrClient.Builder} class directly */ public static CloudSolrClient getCloudSolrClient(String zkHost) { @@ -2270,7 +2279,18 @@ public abstract class SolrTestCaseJ4 extends LuceneTestCase { .withZkHost(zkHost) .build(); } - + + /** + * This method may randomize unspecified aspects of the resulting SolrClient. + * Tests that do not wish to have any randomized behavior should use the + * {@link org.apache.solr.client.solrj.impl.CloudSolrClient.Builder} class directly + */ + public static CloudSolrClient getCloudSolrClient(MiniSolrCloudCluster cluster) { + return new CloudSolrClientBuilder() + .withCluster(cluster) + .build(); + } + /** * This method may randomize unspecified aspects of the resulting SolrClient. * Tests that do not wish to have any randomized behavior should use the @@ -2300,7 +2320,11 @@ public abstract class SolrTestCaseJ4 extends LuceneTestCase { .sendUpdatesToAllReplicasInShard() .build(); } - + + public static CloudSolrClientBuilder newCloudSolrClient(String zkHost) { + return (CloudSolrClientBuilder) new CloudSolrClientBuilder().withZkHost(zkHost); + } + /** * This method may randomize unspecified aspects of the resulting SolrClient. * Tests that do not wish to have any randomized behavior should use the