From caca8279586ffecf9e6a6e80515d3fd531d06c85 Mon Sep 17 00:00:00 2001 From: Nazerke Seidan Date: Fri, 25 Jun 2021 11:22:29 +0600 Subject: [PATCH] SOLR-14341: Move configName into DocCollection (state.json) (#17) SolrCloud: move the reference a collection has to a configSet in ZooKeeper from the collections/collectionName into its state.json. For many-collection clusters, this is an optimization when the cluster status is fetched. Co-authored-by: Nazerke Seidan Co-authored-by: David Smiley --- solr/CHANGES.txt | 4 ++ .../org/apache/solr/cloud/ConfigSetCmds.java | 9 +--- .../cloud/DistributedClusterStateUpdater.java | 6 ++- .../apache/solr/cloud/ZkConfigSetService.java | 8 +--- .../cloud/api/collections/AddReplicaCmd.java | 2 +- .../solr/cloud/api/collections/BackupCmd.java | 2 +- .../cloud/api/collections/CollApiCmds.java | 24 +++++++--- .../collections/CollectionHandlingUtils.java | 25 ----------- .../api/collections/CreateCollectionCmd.java | 13 +++--- .../api/collections/DeleteCollectionCmd.java | 9 +--- .../cloud/api/collections/MigrateCmd.java | 2 +- .../api/collections/ReindexCollectionCmd.java | 2 +- .../cloud/overseer/ClusterStateMutator.java | 9 ++++ .../cloud/overseer/CollectionMutator.java | 23 +++++++--- .../solr/handler/admin/ClusterStatus.java | 11 ++--- .../SchemaDesignerConfigSetHelper.java | 2 +- .../solr/rest/ManagedResourceStorage.java | 11 ++--- .../java/org/apache/solr/util/SolrCLI.java | 4 +- .../solr/cloud/ClusterStateMockUtil.java | 2 + .../apache/solr/cloud/ClusterStateTest.java | 7 ++- ...rseerCollectionConfigSetProcessorTest.java | 5 +++ .../cloud/OverseerModifyCollectionTest.java | 17 ++----- .../org/apache/solr/cloud/SliceStateTest.java | 5 ++- .../cloud/TestConfigSetsAPIShareSchema.java | 2 +- .../solr/cloud/TestHashPartitioner.java | 5 ++- .../apache/solr/cloud/ZkControllerTest.java | 44 ------------------ .../CollectionsAPIDistributedZkTest.java | 7 +-- .../SimpleCollectionCreateDeleteTest.java | 6 +-- .../cloud/overseer/ZkStateReaderTest.java | 13 ++++-- .../cloud/overseer/ZkStateWriterTest.java | 14 +++--- .../org/apache/solr/core/CoreSorterTest.java | 5 ++- .../solr/common/cloud/ClusterState.java | 45 +++++++++++++++++++ .../solr/common/cloud/DocCollection.java | 14 +++++- .../solr/common/cloud/ZkStateReader.java | 42 +++-------------- .../AbstractCloudBackupRestoreTestCase.java | 3 +- 35 files changed, 191 insertions(+), 211 deletions(-) diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt index d450ae2c69d2..7a56e32da336 100644 --- a/solr/CHANGES.txt +++ b/solr/CHANGES.txt @@ -331,6 +331,10 @@ Other Changes * SOLR-15451: SolrSchema (for Parallel SQL) should use PKI principal for internal request to /admin/luke to get table metadata (Timothy Potter) +* SOLR-14341: SolrCloud: move the reference a collection has to a configSet in ZooKeeper from the + collections/collectionName into its state.json. For many-collection clusters, this is an + optimization when the cluster status is fetched. (Nazerke Seidan, David Smiley) + Bug Fixes --------------------- * SOLR-14546: Fix for a relatively hard to hit issue in OverseerTaskProcessor that could lead to out of order execution diff --git a/solr/core/src/java/org/apache/solr/cloud/ConfigSetCmds.java b/solr/core/src/java/org/apache/solr/cloud/ConfigSetCmds.java index 967463510fb7..0dc9b260d7c6 100644 --- a/solr/core/src/java/org/apache/solr/cloud/ConfigSetCmds.java +++ b/solr/core/src/java/org/apache/solr/cloud/ConfigSetCmds.java @@ -35,7 +35,6 @@ import org.apache.solr.core.ConfigSetProperties; import org.apache.solr.core.ConfigSetService; import org.apache.solr.core.CoreContainer; -import org.apache.zookeeper.KeeperException; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -181,13 +180,7 @@ private static void deleteConfigSet(String configSetName, boolean force, CoreCon ZkStateReader zkStateReader = coreContainer.getZkController().getZkStateReader(); for (Map.Entry entry : zkStateReader.getClusterState().getCollectionsMap().entrySet()) { - String configName = null; - try { - configName = zkStateReader.readConfigName(entry.getKey()); - } catch (KeeperException ex) { - throw new SolrException(SolrException.ErrorCode.BAD_REQUEST, - "Can not delete ConfigSet as it is currently being used by collection [" + entry.getKey() + "]"); - } + String configName = entry.getValue().getConfigName(); if (configSetName.equals(configName)) throw new SolrException(SolrException.ErrorCode.BAD_REQUEST, "Can not delete ConfigSet as it is currently being used by collection [" + entry.getKey() + "]"); diff --git a/solr/core/src/java/org/apache/solr/cloud/DistributedClusterStateUpdater.java b/solr/core/src/java/org/apache/solr/cloud/DistributedClusterStateUpdater.java index 06a7ef9b624d..523fe6b6f4ce 100644 --- a/solr/core/src/java/org/apache/solr/cloud/DistributedClusterStateUpdater.java +++ b/solr/core/src/java/org/apache/solr/cloud/DistributedClusterStateUpdater.java @@ -524,7 +524,11 @@ private ClusterState fetchStateForCollection() throws KeeperException, Interrupt String collectionStatePath = ZkStateReader.getCollectionPath(updater.getCollectionName()); Stat stat = new Stat(); byte[] data = zkStateReader.getZkClient().getData(collectionStatePath, null, stat, true); - ClusterState clusterState = ClusterState.createFromJson(stat.getVersion(), data, Collections.emptySet()); + + // This factory method can detect a missing configName and supply it by reading it from the old ZK location. + // TODO in Solr 10 remove that factory method + ClusterState clusterState = ClusterState.createFromJsonSupportingLegacyConfigName( + stat.getVersion(), data, Collections.emptySet(), updater.getCollectionName(), zkStateReader.getZkClient()); return clusterState; } } diff --git a/solr/core/src/java/org/apache/solr/cloud/ZkConfigSetService.java b/solr/core/src/java/org/apache/solr/cloud/ZkConfigSetService.java index 3c69861aafeb..604b9cc9afda 100644 --- a/solr/core/src/java/org/apache/solr/cloud/ZkConfigSetService.java +++ b/solr/core/src/java/org/apache/solr/cloud/ZkConfigSetService.java @@ -93,13 +93,9 @@ public SolrResourceLoader createCoreResourceLoader(CoreDescriptor cd) { } // The configSet is read from ZK and populated. Ignore CD's pre-existing configSet; only populated in standalone - final String configSetName; - try { - configSetName = zkController.getZkStateReader().readConfigName(colName); + String configSetName = zkController.getClusterState().getCollection(colName).getConfigName(); cd.setConfigSet(configSetName); - } catch (KeeperException ex) { - throw new ZooKeeperException(SolrException.ErrorCode.SERVER_ERROR, "Trouble resolving configSet for collection " + colName + ": " + ex.getMessage()); - } + return new ZkSolrResourceLoader(cd.getInstanceDir(), configSetName, parentLoader.getClassLoader(), zkController); } diff --git a/solr/core/src/java/org/apache/solr/cloud/api/collections/AddReplicaCmd.java b/solr/core/src/java/org/apache/solr/cloud/api/collections/AddReplicaCmd.java index 6d4522116a9c..427d51a39f14 100644 --- a/solr/core/src/java/org/apache/solr/cloud/api/collections/AddReplicaCmd.java +++ b/solr/core/src/java/org/apache/solr/cloud/api/collections/AddReplicaCmd.java @@ -227,7 +227,7 @@ private ModifiableSolrParams getReplicaParams(ClusterState clusterState, ZkNodeP params.set(CoreAdminParams.CORE_NODE_NAME, CollectionHandlingUtils.waitToSeeReplicasInState(ccc.getZkStateReader(), ccc.getSolrCloudManager().getTimeSource(), collectionName, Collections.singleton(createReplica.coreName)).get(createReplica.coreName).getName()); - String configName = zkStateReader.readConfigName(collectionName); + String configName = coll.getConfigName(); String routeKey = message.getStr(ShardParams._ROUTE_); String dataDir = message.getStr(CoreAdminParams.DATA_DIR); String ulogDir = message.getStr(CoreAdminParams.ULOG_DIR); diff --git a/solr/core/src/java/org/apache/solr/cloud/api/collections/BackupCmd.java b/solr/core/src/java/org/apache/solr/cloud/api/collections/BackupCmd.java index 10abe3952d62..98253776bd34 100644 --- a/solr/core/src/java/org/apache/solr/cloud/api/collections/BackupCmd.java +++ b/solr/core/src/java/org/apache/solr/cloud/api/collections/BackupCmd.java @@ -81,7 +81,7 @@ public void call(ClusterState state, ZkNodeProps message, @SuppressWarnings({"ra String backupName = message.getStr(NAME); String repo = message.getStr(CoreAdminParams.BACKUP_REPOSITORY); boolean incremental = message.getBool(CoreAdminParams.BACKUP_INCREMENTAL, true); - String configName = ccc.getZkStateReader().readConfigName(collectionName); + String configName = ccc.getSolrCloudManager().getClusterStateProvider().getCollection(collectionName).getConfigName(); BackupProperties backupProperties = BackupProperties.create(backupName, collectionName, extCollectionName, configName); diff --git a/solr/core/src/java/org/apache/solr/cloud/api/collections/CollApiCmds.java b/solr/core/src/java/org/apache/solr/cloud/api/collections/CollApiCmds.java index fa02123a19d3..97349b17a2d8 100644 --- a/solr/core/src/java/org/apache/solr/cloud/api/collections/CollApiCmds.java +++ b/solr/core/src/java/org/apache/solr/cloud/api/collections/CollApiCmds.java @@ -36,7 +36,6 @@ import org.apache.solr.common.cloud.UrlScheme; import org.apache.solr.common.cloud.ZkNodeProps; import org.apache.solr.common.cloud.ZkStateReader; -import org.apache.solr.common.params.CollectionAdminParams; import org.apache.solr.common.params.CollectionParams; import org.apache.solr.common.params.CommonAdminParams; import org.apache.solr.common.params.CoreAdminParams; @@ -50,6 +49,7 @@ import org.slf4j.LoggerFactory; import static org.apache.solr.common.cloud.ZkStateReader.COLLECTION_PROP; +import static org.apache.solr.common.cloud.ZkStateReader.CONFIGNAME_PROP; import static org.apache.solr.common.cloud.ZkStateReader.CORE_NAME_PROP; import static org.apache.solr.common.cloud.ZkStateReader.CORE_NODE_NAME_PROP; import static org.apache.solr.common.cloud.ZkStateReader.ELECTION_NODE_PROP; @@ -59,6 +59,7 @@ import static org.apache.solr.common.cloud.ZkStateReader.REJOIN_AT_HEAD_PROP; import static org.apache.solr.common.cloud.ZkStateReader.REPLICA_PROP; import static org.apache.solr.common.cloud.ZkStateReader.SHARD_ID_PROP; +import static org.apache.solr.common.params.CollectionAdminParams.COLL_CONF; import static org.apache.solr.common.params.CollectionParams.CollectionAction.*; import static org.apache.solr.common.params.CommonAdminParams.ASYNC; import static org.apache.solr.common.params.CommonParams.NAME; @@ -296,14 +297,17 @@ public void call(ClusterState clusterState, ZkNodeProps message, @SuppressWarnin final String collectionName = message.getStr(ZkStateReader.COLLECTION_PROP); //the rest of the processing is based on writing cluster state properties - //remove the property here to avoid any errors down the pipeline due to this property appearing - String configName = (String) message.getProperties().remove(CollectionAdminParams.COLL_CONF); + String configName = (String) message.getProperties().get(COLL_CONF); if (configName != null) { CollectionHandlingUtils.validateConfigOrThrowSolrException(ccc.getCoreContainer().getConfigSetService(), configName); - CollectionHandlingUtils.createConfNode(ccc.getSolrCloudManager().getDistribStateManager(), configName, collectionName); - new ReloadCollectionCmd(ccc).call(clusterState, new ZkNodeProps(NAME, collectionName), results); + // Back-compatibility reason: update configName in old location + // TODO in Solr 10 this code should go away + String collPath = ZkStateReader.COLLECTIONS_ZKNODE + "/" + collectionName; + if (ccc.getSolrCloudManager().getDistribStateManager().hasData(collPath)) { + ccc.getSolrCloudManager().getDistribStateManager().setData(collPath, Utils.toJSON(Map.of(ZkStateReader.CONFIGNAME_PROP, configName)), -1); + } } if (ccc.getDistributedClusterStateUpdater().isDistributedStateUpdate()) { @@ -321,6 +325,12 @@ public void call(ClusterState clusterState, ZkNodeProps message, @SuppressWarnin for (Map.Entry updateEntry : message.getProperties().entrySet()) { String updateKey = updateEntry.getKey(); + // update key from collection.configName to configName; + // actual renaming happens in org.apache.solr.cloud.overseer.CollectionMutator#modifyCollection + if (updateKey.equals(COLL_CONF)) { + updateKey = CONFIGNAME_PROP; + } + if (!updateKey.equals(ZkStateReader.COLLECTION_PROP) && !updateKey.equals(Overseer.QUEUE_OPERATION) && !updateKey.equals(CommonAdminParams.ASYNC) @@ -341,8 +351,8 @@ public void call(ClusterState clusterState, ZkNodeProps message, @SuppressWarnin throw new SolrException(SolrException.ErrorCode.SERVER_ERROR, "Failed to modify collection", e); } - // if switching to/from read-only mode reload the collection - if (message.keySet().contains(ZkStateReader.READ_ONLY)) { + // if switching to/from read-only mode or configName is not null reload the collection + if (message.keySet().contains(ZkStateReader.READ_ONLY) || configName != null) { new ReloadCollectionCmd(ccc).call(clusterState, new ZkNodeProps(NAME, collectionName), results); } } diff --git a/solr/core/src/java/org/apache/solr/cloud/api/collections/CollectionHandlingUtils.java b/solr/core/src/java/org/apache/solr/cloud/api/collections/CollectionHandlingUtils.java index f56c9aad3715..7adb527690e5 100644 --- a/solr/core/src/java/org/apache/solr/cloud/api/collections/CollectionHandlingUtils.java +++ b/solr/core/src/java/org/apache/solr/cloud/api/collections/CollectionHandlingUtils.java @@ -34,9 +34,6 @@ import org.apache.solr.client.solrj.SolrResponse; import org.apache.solr.client.solrj.SolrServerException; -import org.apache.solr.client.solrj.cloud.AlreadyExistsException; -import org.apache.solr.client.solrj.cloud.BadVersionException; -import org.apache.solr.client.solrj.cloud.DistribStateManager; import org.apache.solr.client.solrj.impl.BaseHttpSolrClient; import org.apache.solr.client.solrj.impl.HttpSolrClient; import org.apache.solr.client.solrj.request.AbstractUpdateRequest; @@ -44,7 +41,6 @@ import org.apache.solr.client.solrj.response.UpdateResponse; import org.apache.solr.cloud.DistributedClusterStateUpdater; import org.apache.solr.cloud.Overseer; -import org.apache.solr.cloud.ZkController; import org.apache.solr.cloud.overseer.ClusterStateMutator; import org.apache.solr.cloud.overseer.OverseerAction; import org.apache.solr.common.SolrException; @@ -64,7 +60,6 @@ import org.apache.solr.handler.component.ShardHandlerFactory; import org.apache.solr.handler.component.ShardRequest; import org.apache.solr.handler.component.ShardResponse; -import org.apache.zookeeper.CreateMode; import org.apache.zookeeper.KeeperException; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -307,26 +302,6 @@ static void validateConfigOrThrowSolrException(ConfigSetService configSetService } } - /** - * This doesn't validate the config (path) itself and is just responsible for creating the confNode. - * That check should be done before the config node is created. - */ - public static void createConfNode(DistribStateManager stateManager, String configName, String coll) throws IOException, AlreadyExistsException, BadVersionException, KeeperException, InterruptedException { - - if (configName != null) { - String collDir = ZkStateReader.COLLECTIONS_ZKNODE + "/" + coll; - log.debug("creating collections conf node {} ", collDir); - byte[] data = Utils.toJSON(Map.of(ZkController.CONFIGNAME_PROP, configName)); - if (stateManager.hasData(collDir)) { - stateManager.setData(collDir, data, -1); - } else { - stateManager.makePath(collDir, data, CreateMode.PERSISTENT, false); - } - } else { - throw new SolrException(SolrException.ErrorCode.BAD_REQUEST,"Unable to get config name"); - } - } - /** * Send request to all replicas of a collection * @return List of replicas which is not live for receiving the request diff --git a/solr/core/src/java/org/apache/solr/cloud/api/collections/CreateCollectionCmd.java b/solr/core/src/java/org/apache/solr/cloud/api/collections/CreateCollectionCmd.java index 4dab2e8295ae..9a150b49763f 100644 --- a/solr/core/src/java/org/apache/solr/cloud/api/collections/CreateCollectionCmd.java +++ b/solr/core/src/java/org/apache/solr/cloud/api/collections/CreateCollectionCmd.java @@ -57,6 +57,7 @@ import org.apache.solr.common.util.Utils; import org.apache.solr.core.ConfigSetService; import org.apache.solr.core.CoreContainer; +import org.apache.solr.handler.admin.ConfigSetsHandler; import org.apache.solr.handler.component.ShardHandler; import org.apache.solr.handler.component.ShardRequest; import org.apache.solr.util.TimeOut; @@ -126,9 +127,7 @@ public void call(ClusterState clusterState, ZkNodeProps message, @SuppressWarnin final String async = message.getStr(ASYNC); ZkStateReader zkStateReader = ccc.getZkStateReader(); - - // this also creates the collection zk node as a side-effect - CollectionHandlingUtils.createConfNode(ccc.getSolrCloudManager().getDistribStateManager(), configName, collectionName); + message.getProperties().put(COLL_CONF, configName); Map collectionParams = new HashMap<>(); Map collectionProps = message.getProperties(); @@ -369,8 +368,9 @@ public void call(ClusterState clusterState, ZkNodeProps message, @SuppressWarnin } else { log.debug("Finished create command on all shards for collection: {}", collectionName); // Emit a warning about production use of data driven functionality + // Note: isAutoGeneratedConfigSet is always a clone of the _default configset boolean defaultConfigSetUsed = message.getStr(COLL_CONF) == null || - message.getStr(COLL_CONF).equals(DEFAULT_CONFIGSET_NAME); + message.getStr(COLL_CONF).equals(DEFAULT_CONFIGSET_NAME) || ConfigSetsHandler.isAutoGeneratedConfigSet(message.getStr(COLL_CONF)); if (defaultConfigSetUsed) { results.add("warning", "Using _default configset. Data driven schema functionality" + " is enabled by default, which is NOT RECOMMENDED for production use. To turn it off:" @@ -560,9 +560,10 @@ public static void createCollectionZkNode(DistribStateManager stateManager, Stri } collectionProps.remove(ZkStateReader.NUM_SHARDS_PROP); // we don't put numShards in the collections properties + collectionProps.remove(ZkStateReader.CONFIGNAME_PROP); // we don't write configName on a zk collection node - ZkNodeProps zkProps = new ZkNodeProps(collectionProps); - stateManager.makePath(collectionPath, Utils.toJSON(zkProps), CreateMode.PERSISTENT, false); + // create a node + stateManager.makePath(collectionPath); } catch (KeeperException e) { //TODO shouldn't the stateManager ensure this does not happen; should throw AlreadyExistsException diff --git a/solr/core/src/java/org/apache/solr/cloud/api/collections/DeleteCollectionCmd.java b/solr/core/src/java/org/apache/solr/cloud/api/collections/DeleteCollectionCmd.java index 6f077025f8eb..77e60e59a4a6 100644 --- a/solr/core/src/java/org/apache/solr/cloud/api/collections/DeleteCollectionCmd.java +++ b/solr/core/src/java/org/apache/solr/cloud/api/collections/DeleteCollectionCmd.java @@ -155,7 +155,7 @@ public void call(ClusterState state, ZkNodeProps message, @SuppressWarnings({"ra } // delete related config set iff: it is auto generated AND not related to any other collection - String configSetName = zkStateReader.readConfigName(collection); + String configSetName = coll.getConfigName(); if (ConfigSetsHandler.isAutoGeneratedConfigSet(configSetName)) { boolean configSetIsUsedByOtherCollection = false; @@ -163,12 +163,7 @@ public void call(ClusterState state, ZkNodeProps message, @SuppressWarnings({"ra // make sure the configSet is not shared with other collections // Similar to what happens in: ConfigSetCmds::deleteConfigSet for (Map.Entry entry : zkStateReader.getClusterState().getCollectionsMap().entrySet()) { - String otherConfigSetName = null; - try { - otherConfigSetName = zkStateReader.readConfigName(entry.getKey()); - } catch (KeeperException ex) { - // ignore 'no config found' errors - } + String otherConfigSetName = entry.getValue().getConfigName(); if (configSetName.equals(otherConfigSetName)) { configSetIsUsedByOtherCollection = true; break; diff --git a/solr/core/src/java/org/apache/solr/cloud/api/collections/MigrateCmd.java b/solr/core/src/java/org/apache/solr/cloud/api/collections/MigrateCmd.java index d17f28fa81ec..746488fcba9e 100644 --- a/solr/core/src/java/org/apache/solr/cloud/api/collections/MigrateCmd.java +++ b/solr/core/src/java/org/apache/solr/cloud/api/collections/MigrateCmd.java @@ -230,7 +230,7 @@ private void migrateKey(ClusterState clusterState, DocCollection sourceCollectio Replica sourceLeader = zkStateReader.getLeaderRetry(sourceCollection.getName(), sourceSlice.getName(), 10000); // create a temporary collection with just one node on the shard leader - String configName = zkStateReader.readConfigName(sourceCollection.getName()); + String configName = sourceCollection.getConfigName(); Map props = Utils.makeMap( Overseer.QUEUE_OPERATION, CREATE.toLower(), NAME, tempSourceCollectionName, diff --git a/solr/core/src/java/org/apache/solr/cloud/api/collections/ReindexCollectionCmd.java b/solr/core/src/java/org/apache/solr/cloud/api/collections/ReindexCollectionCmd.java index 50fe79cf0117..2b3c64742b66 100644 --- a/solr/core/src/java/org/apache/solr/cloud/api/collections/ReindexCollectionCmd.java +++ b/solr/core/src/java/org/apache/solr/cloud/api/collections/ReindexCollectionCmd.java @@ -243,7 +243,7 @@ public void call(ClusterState clusterState, ZkNodeProps message, @SuppressWarnin router = DocRouter.DEFAULT; } - String configName = message.getStr(ZkStateReader.CONFIGNAME_PROP, ccc.getZkStateReader().readConfigName(collection)); + String configName = message.getStr(ZkStateReader.CONFIGNAME_PROP, coll.getConfigName()); String targetCollection; int seq = tmpCollectionSeq.getAndIncrement(); if (sameTarget) { diff --git a/solr/core/src/java/org/apache/solr/cloud/overseer/ClusterStateMutator.java b/solr/core/src/java/org/apache/solr/cloud/overseer/ClusterStateMutator.java index e70c4552d774..a5b58bc5da39 100644 --- a/solr/core/src/java/org/apache/solr/cloud/overseer/ClusterStateMutator.java +++ b/solr/core/src/java/org/apache/solr/cloud/overseer/ClusterStateMutator.java @@ -36,6 +36,7 @@ import org.apache.solr.common.cloud.Slice; import org.apache.solr.common.cloud.ZkNodeProps; import org.apache.solr.common.cloud.ZkStateReader; +import org.apache.solr.common.params.CollectionAdminParams; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -109,6 +110,14 @@ public ZkWriteCommand createCollection(ClusterState clusterState, ZkNodeProps me collectionProps.put("autoCreated", "true"); } + // put configName in props so that it will appear in state.json + final String configName = (String) message.getProperties().get(CollectionAdminParams.COLL_CONF); + + if (configName != null) { + collectionProps.put(ZkStateReader.CONFIGNAME_PROP, configName); + } + + assert !collectionProps.containsKey(CollectionAdminParams.COLL_CONF); DocCollection newCollection = new DocCollection(cName, slices, collectionProps, router, -1); return new ZkWriteCommand(cName, newCollection); diff --git a/solr/core/src/java/org/apache/solr/cloud/overseer/CollectionMutator.java b/solr/core/src/java/org/apache/solr/cloud/overseer/CollectionMutator.java index 472d1b2c52f0..6265fdd7804f 100644 --- a/solr/core/src/java/org/apache/solr/cloud/overseer/CollectionMutator.java +++ b/solr/core/src/java/org/apache/solr/cloud/overseer/CollectionMutator.java @@ -30,8 +30,10 @@ import org.slf4j.LoggerFactory; import static org.apache.solr.common.cloud.ZkStateReader.COLLECTION_PROP; +import static org.apache.solr.common.cloud.ZkStateReader.CONFIGNAME_PROP; import static org.apache.solr.common.cloud.ZkStateReader.NRT_REPLICAS; import static org.apache.solr.common.cloud.ZkStateReader.REPLICATION_FACTOR; +import static org.apache.solr.common.params.CollectionAdminParams.COLL_CONF; public class CollectionMutator { private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass()); @@ -98,7 +100,7 @@ public ZkWriteCommand deleteShard(final ClusterState clusterState, ZkNodeProps m public ZkWriteCommand modifyCollection(final ClusterState clusterState, ZkNodeProps message) { if (!checkCollectionKeyExistence(message)) return ZkStateWriter.NO_OP; DocCollection coll = clusterState.getCollection(message.getStr(COLLECTION_PROP)); - Map m = coll.shallowCopy(); + Map props = coll.shallowCopy(); boolean hasAnyOps = false; PerReplicaStatesOps replicaOps = null; for (String prop : CollectionAdminRequest.MODIFIABLE_COLLECTION_PROPERTIES) { @@ -118,12 +120,17 @@ public ZkWriteCommand modifyCollection(final ClusterState clusterState, ZkNodePr if (message.containsKey(prop)) { hasAnyOps = true; if (message.get(prop) == null) { - m.remove(prop); + props.remove(prop); } else { - m.put(prop, message.get(prop)); + // rename key from collection.configName to configName + if (prop.equals(COLL_CONF)) { + props.put(CONFIGNAME_PROP, message.get(prop)); + } else { + props.put(prop, message.get(prop)); + } } if (prop == REPLICATION_FACTOR) { //SOLR-11676 : keep NRT_REPLICAS and REPLICATION_FACTOR in sync - m.put(NRT_REPLICAS, message.get(REPLICATION_FACTOR)); + props.put(NRT_REPLICAS, message.get(REPLICATION_FACTOR)); } } } @@ -132,9 +139,9 @@ public ZkWriteCommand modifyCollection(final ClusterState clusterState, ZkNodePr if (prop.startsWith(CollectionAdminRequest.PROPERTY_PREFIX)) { hasAnyOps = true; if (message.get(prop) == null) { - m.remove(prop); + props.remove(prop); } else { - m.put(prop, message.get(prop)); + props.put(prop, message.get(prop)); } } } @@ -143,7 +150,9 @@ public ZkWriteCommand modifyCollection(final ClusterState clusterState, ZkNodePr return ZkStateWriter.NO_OP; } - DocCollection collection = new DocCollection(coll.getName(), coll.getSlicesMap(), m, coll.getRouter(), coll.getZNodeVersion()); + assert !props.containsKey(COLL_CONF); + + DocCollection collection = new DocCollection(coll.getName(), coll.getSlicesMap(), props, coll.getRouter(), coll.getZNodeVersion()); if (replicaOps == null){ return new ZkWriteCommand(coll.getName(), collection); } else { 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 b95c3c24345f..c314240e4b1b 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 @@ -183,14 +183,9 @@ public void getClusterStatus(@SuppressWarnings({"rawtypes"})NamedList results) if (collectionVsAliases.containsKey(name) && !collectionVsAliases.get(name).isEmpty()) { collectionStatus.put("aliases", collectionVsAliases.get(name)); } - try { - String configName = zkStateReader.readConfigName(name); - collectionStatus.put("configName", configName); - collectionProps.add(name, collectionStatus); - } catch (KeeperException.NoNodeException ex) { - // skip this collection because the configset's znode has been deleted - // which can happen during aggressive collection removal, see SOLR-10720 - } + String configName = clusterStateCollection.getConfigName(); + collectionStatus.put("configName", configName); + collectionProps.add(name, collectionStatus); } List liveNodes = zkStateReader.getZkClient().getChildren(ZkStateReader.LIVE_NODES_ZKNODE, null, true); diff --git a/solr/core/src/java/org/apache/solr/handler/designer/SchemaDesignerConfigSetHelper.java b/solr/core/src/java/org/apache/solr/handler/designer/SchemaDesignerConfigSetHelper.java index 2b768b61c0bc..ee71a0056fba 100644 --- a/solr/core/src/java/org/apache/solr/handler/designer/SchemaDesignerConfigSetHelper.java +++ b/solr/core/src/java/org/apache/solr/handler/designer/SchemaDesignerConfigSetHelper.java @@ -172,7 +172,7 @@ List listCollectionsForConfig(String configSet) { } try { - if (configSet.equals(zkStateReader().readConfigName(coll)) && e.getValue().get() != null) { + if (configSet.equals(e.getValue().get().getConfigName()) && e.getValue().get() != null) { collections.add(coll); } } catch (Exception exc) { diff --git a/solr/core/src/java/org/apache/solr/rest/ManagedResourceStorage.java b/solr/core/src/java/org/apache/solr/rest/ManagedResourceStorage.java index e9102a299daf..382c49ac6531 100644 --- a/solr/core/src/java/org/apache/solr/rest/ManagedResourceStorage.java +++ b/solr/core/src/java/org/apache/solr/rest/ManagedResourceStorage.java @@ -41,6 +41,7 @@ import org.apache.solr.common.SolrException; import org.apache.solr.common.SolrException.ErrorCode; import org.apache.solr.common.cloud.SolrZkClient; +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.core.SolrResourceLoader; @@ -86,18 +87,18 @@ public static StorageIO newStorageIO(String collection, SolrResourceLoader resou StorageIO storageIO; SolrZkClient zkClient = null; - String zkConfigName = null; + String configName = null; if (resourceLoader instanceof ZkSolrResourceLoader) { zkClient = ((ZkSolrResourceLoader)resourceLoader).getZkController().getZkClient(); try { - zkConfigName = ((ZkSolrResourceLoader)resourceLoader).getZkController(). - getZkStateReader().readConfigName(collection); + final ZkStateReader zkStateReader = ((ZkSolrResourceLoader)resourceLoader).getZkController().getZkStateReader(); + configName = zkStateReader.getClusterState().getCollection(collection).getConfigName(); } catch (Exception e) { log.error("Failed to get config name due to", e); throw new SolrException(ErrorCode.SERVER_ERROR, "Failed to load config name for collection:" + collection + " due to: ", e); } - if (zkConfigName == null) { + if (configName == null) { throw new SolrException(ErrorCode.SERVER_ERROR, "Could not find config name for collection:" + collection); } @@ -107,7 +108,7 @@ public static StorageIO newStorageIO(String collection, SolrResourceLoader resou storageIO = resourceLoader.newInstance(initArgs.get(STORAGE_IO_CLASS_INIT_ARG), StorageIO.class); } else { if (zkClient != null) { - String znodeBase = "/configs/"+zkConfigName; + String znodeBase = "/configs/"+configName; log.debug("Setting up ZooKeeper-based storage for the RestManager with znodeBase: {}", znodeBase); storageIO = new ManagedResourceStorage.ZooKeeperStorageIO(zkClient, znodeBase); } else { diff --git a/solr/core/src/java/org/apache/solr/util/SolrCLI.java b/solr/core/src/java/org/apache/solr/util/SolrCLI.java index 7cc2ed327950..e297e2173157 100755 --- a/solr/core/src/java/org/apache/solr/util/SolrCLI.java +++ b/solr/core/src/java/org/apache/solr/util/SolrCLI.java @@ -2390,7 +2390,7 @@ protected void deleteCollection(CloudSolrClient cloudSolrClient, CommandLine cli throw new IllegalArgumentException("Collection "+collectionName+" not found!"); } - String configName = zkStateReader.readConfigName(collectionName); + String configName = zkStateReader.getClusterState().getCollection(collectionName).getConfigName(); boolean deleteConfig = "true".equals(cli.getOptionValue("deleteConfig", "true")); if (deleteConfig && configName != null) { if (cli.hasOption("forceDeleteConfig")) { @@ -2409,7 +2409,7 @@ protected void deleteCollection(CloudSolrClient cloudSolrClient, CommandLine cli if (collectionName.equals(next)) continue; // don't check the collection we're deleting - if (configName.equals(zkStateReader.readConfigName(next))) { + if (configName.equals(zkStateReader.getClusterState().getCollection(next).getConfigName())) { deleteConfig = false; log.warn("Configuration directory {} is also being used by {}{}" , configName, next diff --git a/solr/core/src/test/org/apache/solr/cloud/ClusterStateMockUtil.java b/solr/core/src/test/org/apache/solr/cloud/ClusterStateMockUtil.java index 1ffe0cc6fa48..297ec4f1d2f0 100644 --- a/solr/core/src/test/org/apache/solr/cloud/ClusterStateMockUtil.java +++ b/solr/core/src/test/org/apache/solr/cloud/ClusterStateMockUtil.java @@ -33,6 +33,7 @@ import org.apache.solr.common.cloud.Slice; import org.apache.solr.common.cloud.ZkStateReader; import org.apache.solr.common.util.Utils; +import org.apache.solr.handler.admin.ConfigSetsHandler; /** * A utility class that can create mock ZkStateReader objects with custom ClusterState objects created @@ -125,6 +126,7 @@ public static ZkStateReader buildClusterState(String clusterDescription, int rep switch (m.group(1)) { case "c": slices = new HashMap<>(); + collectionProps.put(ZkStateReader.CONFIGNAME_PROP, ConfigSetsHandler.DEFAULT_CONFIGSET_NAME); docCollection = new DocCollection(collName = "collection" + (collectionStates.size() + 1), slices, collectionProps, DocRouter.DEFAULT); collectionStates.put(docCollection.getName(), docCollection); break; diff --git a/solr/core/src/test/org/apache/solr/cloud/ClusterStateTest.java b/solr/core/src/test/org/apache/solr/cloud/ClusterStateTest.java index f6e74da40624..a8a13ec19ae1 100644 --- a/solr/core/src/test/org/apache/solr/cloud/ClusterStateTest.java +++ b/solr/core/src/test/org/apache/solr/cloud/ClusterStateTest.java @@ -27,7 +27,9 @@ import org.apache.solr.common.cloud.DocRouter; import org.apache.solr.common.cloud.Replica; import org.apache.solr.common.cloud.Slice; +import org.apache.solr.common.cloud.ZkStateReader; import org.apache.solr.common.util.Utils; +import org.apache.solr.handler.admin.ConfigSetsHandler; import org.junit.Test; public class ClusterStateTest extends SolrTestCaseJ4 { @@ -47,14 +49,15 @@ public void testStoreAndRead() throws Exception { props.put("prop1", "value"); props.put("prop2", "value2"); + props.put(ZkStateReader.CONFIGNAME_PROP, ConfigSetsHandler.DEFAULT_CONFIGSET_NAME); Replica replica = new Replica("node1", props, "collection1", "shard1"); sliceToProps.put("node1", replica); Slice slice = new Slice("shard1", sliceToProps, null, "collection1"); slices.put("shard1", slice); Slice slice2 = new Slice("shard2", sliceToProps, null, "collection1"); slices.put("shard2", slice2); - collectionStates.put("collection1", new DocCollection("collection1", slices, null, DocRouter.DEFAULT)); - collectionStates.put("collection2", new DocCollection("collection2", slices, null, DocRouter.DEFAULT)); + collectionStates.put("collection1", new DocCollection("collection1", slices, props, DocRouter.DEFAULT)); + collectionStates.put("collection2", new DocCollection("collection2", slices, props, DocRouter.DEFAULT)); ClusterState clusterState = new ClusterState(liveNodes, collectionStates); byte[] bytes = Utils.toJSON(clusterState); diff --git a/solr/core/src/test/org/apache/solr/cloud/OverseerCollectionConfigSetProcessorTest.java b/solr/core/src/test/org/apache/solr/cloud/OverseerCollectionConfigSetProcessorTest.java index ef869c9071d7..eca25e294464 100644 --- a/solr/core/src/test/org/apache/solr/cloud/OverseerCollectionConfigSetProcessorTest.java +++ b/solr/core/src/test/org/apache/solr/cloud/OverseerCollectionConfigSetProcessorTest.java @@ -53,6 +53,7 @@ import org.apache.solr.common.cloud.SolrZkClient; import org.apache.solr.common.cloud.ZkNodeProps; import org.apache.solr.common.cloud.ZkStateReader; +import org.apache.solr.common.params.CollectionAdminParams; import org.apache.solr.common.params.CollectionParams; import org.apache.solr.common.params.CoreAdminParams; import org.apache.solr.common.params.CoreAdminParams.CoreAdminAction; @@ -564,6 +565,10 @@ private void handleCreateCollMessageProps(ZkNodeProps props) { try { if (CollectionParams.CollectionAction.CREATE.isEqual(props.getStr("operation"))) { String collName = props.getStr("name"); + if (props.containsKey(CollectionAdminParams.COLL_CONF)) { + String configName = (String) props.getProperties().remove(CollectionAdminParams.COLL_CONF); + props.getProperties().put(ZkStateReader.CONFIGNAME_PROP, configName); + } if (collName != null) collectionsSet.put(collName, new ClusterState.CollectionRef( new DocCollection(collName, new HashMap<>(), props.getProperties(), DocRouter.DEFAULT))); } diff --git a/solr/core/src/test/org/apache/solr/cloud/OverseerModifyCollectionTest.java b/solr/core/src/test/org/apache/solr/cloud/OverseerModifyCollectionTest.java index e3c42055f7af..a1411947c980 100644 --- a/solr/core/src/test/org/apache/solr/cloud/OverseerModifyCollectionTest.java +++ b/solr/core/src/test/org/apache/solr/cloud/OverseerModifyCollectionTest.java @@ -18,17 +18,14 @@ package org.apache.solr.cloud; import java.util.Collections; -import java.util.Map; import org.apache.solr.client.solrj.request.CollectionAdminRequest; import org.apache.solr.client.solrj.response.RequestStatusState; -import org.apache.solr.common.cloud.ZkStateReader; -import org.apache.solr.common.util.Utils; -import org.apache.zookeeper.KeeperException; import org.junit.BeforeClass; import org.junit.Test; + public class OverseerModifyCollectionTest extends SolrCloudTestCase { @BeforeClass @@ -53,7 +50,8 @@ public void testModifyColl() throws Exception { .processAndWait(cluster.getSolrClient(), DEFAULT_TIMEOUT); assertEquals(requestStatusState, RequestStatusState.COMPLETED); - assertEquals("conf2", getConfigNameFromZk(collName)); + String configName = cluster.getSolrClient().getClusterStateProvider().getCollection(collName).getConfigName(); + assertEquals("conf2", configName); //Try an invalid config name Exception e = expectThrows(Exception.class, () -> { @@ -63,14 +61,5 @@ public void testModifyColl() throws Exception { }); assertTrue(e.getMessage(), e.getMessage().contains("Can not find the specified config set")); - } - - private String getConfigNameFromZk(String collName) throws KeeperException, InterruptedException { - byte[] b = zkClient().getData(ZkStateReader.getCollectionPathRoot(collName), null, null, false); - @SuppressWarnings({"rawtypes"}) - Map confData = (Map) Utils.fromJSON(b); - return (String) confData.get(ZkController.CONFIGNAME_PROP); - } - } diff --git a/solr/core/src/test/org/apache/solr/cloud/SliceStateTest.java b/solr/core/src/test/org/apache/solr/cloud/SliceStateTest.java index 7a3d02a02413..8c2d5c3cf6b0 100644 --- a/solr/core/src/test/org/apache/solr/cloud/SliceStateTest.java +++ b/solr/core/src/test/org/apache/solr/cloud/SliceStateTest.java @@ -27,7 +27,9 @@ import org.apache.solr.common.cloud.DocRouter; import org.apache.solr.common.cloud.Replica; import org.apache.solr.common.cloud.Slice; +import org.apache.solr.common.cloud.ZkStateReader; import org.apache.solr.common.util.Utils; +import org.apache.solr.handler.admin.ConfigSetsHandler; import org.junit.Test; public class SliceStateTest extends SolrTestCaseJ4 { @@ -43,13 +45,14 @@ public void testDefaultSliceState() { Map props = new HashMap<>(); props.put("node_name", "127.0.0.1:10000_solr"); props.put("core", "core1"); + props.put(ZkStateReader.CONFIGNAME_PROP, ConfigSetsHandler.DEFAULT_CONFIGSET_NAME); Replica replica = new Replica("node1", props, "collection1", "shard1"); sliceToProps.put("node1", replica); Slice slice = new Slice("shard1", sliceToProps, null, "collection1"); assertSame("Default state not set to active", Slice.State.ACTIVE, slice.getState()); slices.put("shard1", slice); - collectionStates.put("collection1", new DocCollection("collection1", slices, null, DocRouter.DEFAULT)); + collectionStates.put("collection1", new DocCollection("collection1", slices, props, DocRouter.DEFAULT)); ClusterState clusterState = new ClusterState(liveNodes, collectionStates); byte[] bytes = Utils.toJSON(clusterState); diff --git a/solr/core/src/test/org/apache/solr/cloud/TestConfigSetsAPIShareSchema.java b/solr/core/src/test/org/apache/solr/cloud/TestConfigSetsAPIShareSchema.java index 8d0b84af0e4c..81284b767c55 100644 --- a/solr/core/src/test/org/apache/solr/cloud/TestConfigSetsAPIShareSchema.java +++ b/solr/core/src/test/org/apache/solr/cloud/TestConfigSetsAPIShareSchema.java @@ -81,7 +81,7 @@ public void testSharedSchema() throws Exception { // change col1's configSet CollectionAdminRequest.modifyCollection("col1", map("collection.configName", (Object) "conf1") // from cShare - ).processAndWait(cluster.getSolrClient(), DEFAULT_TIMEOUT); + ).process(cluster.getSolrClient(), "col1"); try (SolrCore coreCol1 = coreContainer.getCore("col1_shard1_replica_n1"); SolrCore coreCol2 = coreContainer.getCore("col2_shard1_replica_n1")) { diff --git a/solr/core/src/test/org/apache/solr/cloud/TestHashPartitioner.java b/solr/core/src/test/org/apache/solr/cloud/TestHashPartitioner.java index 55e69fae0f76..fa7c094f768b 100644 --- a/solr/core/src/test/org/apache/solr/cloud/TestHashPartitioner.java +++ b/solr/core/src/test/org/apache/solr/cloud/TestHashPartitioner.java @@ -17,6 +17,7 @@ package org.apache.solr.cloud; import java.util.Collection; +import java.util.Collections; import java.util.HashMap; import java.util.HashSet; import java.util.List; @@ -30,8 +31,10 @@ import org.apache.solr.common.cloud.DocRouter.Range; import org.apache.solr.common.cloud.PlainIdRouter; import org.apache.solr.common.cloud.Slice; +import org.apache.solr.common.cloud.ZkStateReader; import org.apache.solr.common.util.Hash; import org.apache.solr.common.util.StrUtils; +import org.apache.solr.handler.admin.ConfigSetsHandler; public class TestHashPartitioner extends SolrTestCaseJ4 { @@ -277,7 +280,7 @@ DocCollection createCollection(int nSlices, DocRouter router) { slices.put(slice.getName(), slice); } - DocCollection coll = new DocCollection("collection1", slices, null, router); + DocCollection coll = new DocCollection("collection1", slices, Collections.singletonMap(ZkStateReader.CONFIGNAME_PROP, ConfigSetsHandler.DEFAULT_CONFIGSET_NAME), router); return coll; } diff --git a/solr/core/src/test/org/apache/solr/cloud/ZkControllerTest.java b/solr/core/src/test/org/apache/solr/cloud/ZkControllerTest.java index 09be95f4c7e2..8fdc232e4595 100644 --- a/solr/core/src/test/org/apache/solr/cloud/ZkControllerTest.java +++ b/solr/core/src/test/org/apache/solr/cloud/ZkControllerTest.java @@ -20,7 +20,6 @@ import java.util.Collections; import java.util.HashMap; import java.util.List; -import java.util.Map; import java.util.Properties; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicReference; @@ -43,7 +42,6 @@ import org.apache.zookeeper.CreateMode; import org.junit.AfterClass; import org.junit.BeforeClass; -import org.junit.Test; import static org.apache.solr.common.cloud.ZkStateReader.COLLECTION_PROP; import static org.apache.solr.common.cloud.ZkStateReader.SHARD_ID_PROP; @@ -170,48 +168,6 @@ public void testNodeNameUrlConversion() throws Exception { } } - @Test - public void testReadConfigName() throws Exception { - Path zkDir = createTempDir("zkData"); - CoreContainer cc = null; - - ZkTestServer server = new ZkTestServer(zkDir); - try { - server.run(); - - SolrZkClient zkClient = new SolrZkClient(server.getZkAddress(), TIMEOUT); - String actualConfigName = "firstConfig"; - - zkClient.makePath(ZkConfigSetService.CONFIGS_ZKNODE + "/" + actualConfigName, true); - - Map props = new HashMap<>(); - props.put("configName", actualConfigName); - ZkNodeProps zkProps = new ZkNodeProps(props); - zkClient.makePath(ZkStateReader.COLLECTIONS_ZKNODE + "/" - + COLLECTION_NAME, Utils.toJSON(zkProps), - CreateMode.PERSISTENT, true); - - zkClient.close(); - - cc = getCoreContainer(); - - CloudConfig cloudConfig = new CloudConfig.CloudConfigBuilder("127.0.0.1", 8983, "solr").build(); - ZkController zkController = new ZkController(cc, server.getZkAddress(), TIMEOUT, cloudConfig, () -> null); - try { - String configName = zkController.getZkStateReader().readConfigName(COLLECTION_NAME); - assertEquals(configName, actualConfigName); - } finally { - zkController.close(); - } - } finally { - if (cc != null) { - cc.shutdown(); - } - server.shutdown(); - } - - } - public void testGetHostName() throws Exception { Path zkDir = createTempDir("zkData"); CoreContainer cc = null; diff --git a/solr/core/src/test/org/apache/solr/cloud/api/collections/CollectionsAPIDistributedZkTest.java b/solr/core/src/test/org/apache/solr/cloud/api/collections/CollectionsAPIDistributedZkTest.java index 528efa83fbd9..12f0c8323108 100644 --- a/solr/core/src/test/org/apache/solr/cloud/api/collections/CollectionsAPIDistributedZkTest.java +++ b/solr/core/src/test/org/apache/solr/cloud/api/collections/CollectionsAPIDistributedZkTest.java @@ -55,13 +55,11 @@ import org.apache.solr.client.solrj.response.CollectionAdminResponse; import org.apache.solr.client.solrj.response.CoreAdminResponse; import org.apache.solr.cloud.SolrCloudTestCase; -import org.apache.solr.cloud.ZkController; import org.apache.solr.common.SolrException; 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.ZkNodeProps; import org.apache.solr.common.cloud.ZkStateReader; import org.apache.solr.common.params.CollectionParams.CollectionAction; import org.apache.solr.common.params.CoreAdminParams; @@ -336,10 +334,7 @@ public void testDeleteNonExistentCollection() throws Exception { @Test public void testSpecificConfigsets() throws Exception { CollectionAdminRequest.createCollection("withconfigset2", "conf2", 1, 1).process(cluster.getSolrClient()); - byte[] data = zkClient().getData(ZkStateReader.COLLECTIONS_ZKNODE + "/" + "withconfigset2", null, null, true); - assertNotNull(data); - ZkNodeProps props = ZkNodeProps.load(data); - String configName = props.getStr(ZkController.CONFIGNAME_PROP); + String configName = cluster.getSolrClient().getClusterStateProvider().getCollection("withconfigset2").getConfigName(); assertEquals("conf2", configName); } diff --git a/solr/core/src/test/org/apache/solr/cloud/api/collections/SimpleCollectionCreateDeleteTest.java b/solr/core/src/test/org/apache/solr/cloud/api/collections/SimpleCollectionCreateDeleteTest.java index 782a3c037fc3..77431ff371e8 100644 --- a/solr/core/src/test/org/apache/solr/cloud/api/collections/SimpleCollectionCreateDeleteTest.java +++ b/solr/core/src/test/org/apache/solr/cloud/api/collections/SimpleCollectionCreateDeleteTest.java @@ -110,7 +110,7 @@ public void testDeleteAlsoDeletesAutocreatedConfigSet() throws Exception { // collection exists now assertTrue(cloudClient.getZkStateReader().getZkClient().exists(ZkStateReader.COLLECTIONS_ZKNODE + "/" + collectionName, false)); - String configName = cloudClient.getZkStateReader().readConfigName(collectionName); + String configName = cloudClient.getClusterStateProvider().getCollection(collectionName).getConfigName(); // config for this collection is '.AUTOCREATED', and exists globally assertTrue(configName.endsWith(".AUTOCREATED")); @@ -138,7 +138,7 @@ public void testDeleteDoesNotDeleteSharedAutocreatedConfigSet() throws Exception // collection exists now assertTrue(getZkClient().exists(ZkStateReader.COLLECTIONS_ZKNODE + "/" + collectionNameInitial, false)); - String configName = cloudClient.getZkStateReader().readConfigName(collectionNameInitial); + String configName = cloudClient.getClusterStateProvider().getCollection(collectionNameInitial).getConfigName(); // config for this collection is '.AUTOCREATED', and exists globally assertTrue(configName.endsWith(".AUTOCREATED")); @@ -152,7 +152,7 @@ public void testDeleteDoesNotDeleteSharedAutocreatedConfigSet() throws Exception assertTrue("The collection with shared config set should have been created", requestWithSharedConfig.get("success") != null); assertTrue("The new collection should exist after a successful creation", getZkClient().exists(ZkStateReader.COLLECTIONS_ZKNODE + "/" + collectionNameWithSharedConfig, false)); - String configNameOfSecondCollection = cloudClient.getZkStateReader().readConfigName(collectionNameWithSharedConfig); + String configNameOfSecondCollection = cloudClient.getClusterStateProvider().getCollection(collectionNameWithSharedConfig).getConfigName(); assertEquals("Both collections should be using the same config", configName, configNameOfSecondCollection); diff --git a/solr/core/src/test/org/apache/solr/cloud/overseer/ZkStateReaderTest.java b/solr/core/src/test/org/apache/solr/cloud/overseer/ZkStateReaderTest.java index 2bf0971de44f..c73f1528bd96 100644 --- a/solr/core/src/test/org/apache/solr/cloud/overseer/ZkStateReaderTest.java +++ b/solr/core/src/test/org/apache/solr/cloud/overseer/ZkStateReaderTest.java @@ -19,6 +19,7 @@ import java.nio.file.Path; import java.util.Collections; import java.util.HashMap; +import java.util.Map; import java.util.concurrent.TimeUnit; import org.apache.lucene.util.IOUtils; @@ -33,6 +34,7 @@ import org.apache.solr.common.cloud.SolrZkClient; import org.apache.solr.common.cloud.ZkStateReader; import org.apache.solr.common.util.TimeSource; +import org.apache.solr.handler.admin.ConfigSetsHandler; import org.apache.solr.util.TimeOut; public class ZkStateReaderTest extends SolrTestCaseJ4 { @@ -60,7 +62,7 @@ public void testExternalCollectionWatchedNotWatched() throws Exception{ // create new collection ZkWriteCommand c1 = new ZkWriteCommand("c1", - new DocCollection("c1", new HashMap<>(), new HashMap<>(), DocRouter.DEFAULT, 0)); + new DocCollection("c1", new HashMap<>(), Map.of(ZkStateReader.CONFIGNAME_PROP, ConfigSetsHandler.DEFAULT_CONFIGSET_NAME), DocRouter.DEFAULT, 0)); writer.enqueueUpdate(reader.getClusterState(), Collections.singletonList(c1), null); writer.writePendingUpdates(); reader.forceUpdateCollection("c1"); @@ -97,14 +99,17 @@ public void testCollectionStateWatcherCaching() throws Exception { zkClient.makePath(ZkStateReader.COLLECTIONS_ZKNODE + "/c1", true); ZkStateWriter writer = new ZkStateWriter(reader, new Stats()); - DocCollection state = new DocCollection("c1", new HashMap<>(), new HashMap<>(), DocRouter.DEFAULT, 0); + DocCollection state = new DocCollection("c1", new HashMap<>(), Map.of(ZkStateReader.CONFIGNAME_PROP, ConfigSetsHandler.DEFAULT_CONFIGSET_NAME), DocRouter.DEFAULT, 0); ZkWriteCommand wc = new ZkWriteCommand("c1", state); writer.enqueueUpdate(reader.getClusterState(), Collections.singletonList(wc), null); writer.writePendingUpdates(); assertTrue(zkClient.exists(ZkStateReader.COLLECTIONS_ZKNODE + "/c1/state.json", true)); reader.waitForState("c1", 1, TimeUnit.SECONDS, (liveNodes, collectionState) -> collectionState != null); - state = new DocCollection("c1", new HashMap<>(), Collections.singletonMap("x", "y"), DocRouter.DEFAULT, 0); + Map props = new HashMap<>(); + props.put("x", "y"); + props.put(ZkStateReader.CONFIGNAME_PROP, ConfigSetsHandler.DEFAULT_CONFIGSET_NAME); + state = new DocCollection("c1", new HashMap<>(), props, DocRouter.DEFAULT, 0); wc = new ZkWriteCommand("c1", state); writer.enqueueUpdate(reader.getClusterState(), Collections.singletonList(wc), null); writer.writePendingUpdates(); @@ -156,7 +161,7 @@ public void testWatchedCollectionCreation() throws Exception { // create new collection - DocCollection state = new DocCollection("c1", new HashMap<>(), new HashMap<>(), DocRouter.DEFAULT, 0); + DocCollection state = new DocCollection("c1", new HashMap<>(), Map.of(ZkStateReader.CONFIGNAME_PROP, ConfigSetsHandler.DEFAULT_CONFIGSET_NAME), DocRouter.DEFAULT, 0); ZkWriteCommand wc = new ZkWriteCommand("c1", state); writer.enqueueUpdate(reader.getClusterState(), Collections.singletonList(wc), null); writer.writePendingUpdates(); diff --git a/solr/core/src/test/org/apache/solr/cloud/overseer/ZkStateWriterTest.java b/solr/core/src/test/org/apache/solr/cloud/overseer/ZkStateWriterTest.java index 8bc1d79b188e..2692704cafa6 100644 --- a/solr/core/src/test/org/apache/solr/cloud/overseer/ZkStateWriterTest.java +++ b/solr/core/src/test/org/apache/solr/cloud/overseer/ZkStateWriterTest.java @@ -37,6 +37,7 @@ import org.apache.solr.common.cloud.SolrZkClient; import org.apache.solr.common.cloud.ZkStateReader; import org.apache.solr.common.util.Utils; +import org.apache.solr.handler.admin.ConfigSetsHandler; import org.apache.zookeeper.KeeperException; import org.junit.AfterClass; import org.junit.BeforeClass; @@ -80,9 +81,10 @@ public void testZkStateWriterBatching() throws Exception { zkClient.makePath(ZkStateReader.COLLECTIONS_ZKNODE + "/c2", true); zkClient.makePath(ZkStateReader.COLLECTIONS_ZKNODE + "/c3", true); - ZkWriteCommand c1 = new ZkWriteCommand("c1", new DocCollection("c1", new HashMap<>(), new HashMap<>(), DocRouter.DEFAULT, 0)); - ZkWriteCommand c2 = new ZkWriteCommand("c2", new DocCollection("c2", new HashMap<>(), new HashMap<>(), DocRouter.DEFAULT, 0)); - ZkWriteCommand c3 = new ZkWriteCommand("c3", new DocCollection("c3", new HashMap<>(), new HashMap<>(), DocRouter.DEFAULT, 0)); + Map props = Collections.singletonMap(ZkStateReader.CONFIGNAME_PROP, ConfigSetsHandler.DEFAULT_CONFIGSET_NAME); + ZkWriteCommand c1 = new ZkWriteCommand("c1", new DocCollection("c1", new HashMap<>(), props, DocRouter.DEFAULT, 0)); + ZkWriteCommand c2 = new ZkWriteCommand("c2", new DocCollection("c2", new HashMap<>(), props, DocRouter.DEFAULT, 0)); + ZkWriteCommand c3 = new ZkWriteCommand("c3", new DocCollection("c3", new HashMap<>(), props, DocRouter.DEFAULT, 0)); ZkStateWriter writer = new ZkStateWriter(reader, new Stats()); // First write is flushed immediately @@ -129,7 +131,7 @@ public void testSingleExternalCollection() throws Exception { // create new collection ZkWriteCommand c1 = new ZkWriteCommand("c1", - new DocCollection("c1", new HashMap(), new HashMap(), DocRouter.DEFAULT, 0)); + new DocCollection("c1", new HashMap(), Collections.singletonMap(ZkStateReader.CONFIGNAME_PROP, ConfigSetsHandler.DEFAULT_CONFIGSET_NAME), DocRouter.DEFAULT, 0)); writer.enqueueUpdate(reader.getClusterState(), Collections.singletonList(c1), null); writer.writePendingUpdates(); @@ -169,7 +171,7 @@ public void testExternalModification() throws Exception { // create collection 2 ZkWriteCommand c2 = new ZkWriteCommand("c2", - new DocCollection("c2", new HashMap(), new HashMap(), DocRouter.DEFAULT, 0)); + new DocCollection("c2", new HashMap(), Collections.singletonMap(ZkStateReader.CONFIGNAME_PROP, ConfigSetsHandler.DEFAULT_CONFIGSET_NAME), DocRouter.DEFAULT, 0)); state = writer.enqueueUpdate(state, Collections.singletonList(c2), null); assertFalse(writer.hasPendingUpdates()); // first write is flushed immediately @@ -196,7 +198,7 @@ public void testExternalModification() throws Exception { // Will trigger flush Thread.sleep(Overseer.STATE_UPDATE_DELAY+100); ZkWriteCommand c1 = new ZkWriteCommand("c1", - new DocCollection("c1", new HashMap(), new HashMap(), DocRouter.DEFAULT, 0)); + new DocCollection("c1", new HashMap(), Collections.singletonMap(ZkStateReader.CONFIGNAME_PROP, ConfigSetsHandler.DEFAULT_CONFIGSET_NAME), DocRouter.DEFAULT, 0)); try { writer.enqueueUpdate(state, Collections.singletonList(c1), null); diff --git a/solr/core/src/test/org/apache/solr/core/CoreSorterTest.java b/solr/core/src/test/org/apache/solr/core/CoreSorterTest.java index baa8bd4034d2..70fdecc73ebe 100644 --- a/solr/core/src/test/org/apache/solr/core/CoreSorterTest.java +++ b/solr/core/src/test/org/apache/solr/core/CoreSorterTest.java @@ -34,7 +34,9 @@ import org.apache.solr.common.cloud.DocRouter; import org.apache.solr.common.cloud.Replica; import org.apache.solr.common.cloud.Slice; +import org.apache.solr.common.cloud.ZkStateReader; import org.apache.solr.core.CoreSorter.CountsForEachShard; +import org.apache.solr.handler.admin.ConfigSetsHandler; import org.junit.Test; import static org.mockito.Mockito.mock; @@ -124,7 +126,8 @@ public void integrationTest() { Map replicaMap = replicas.stream().collect(Collectors.toMap(Replica::getName, Function.identity())); sliceMap.put(slice, new Slice(slice, replicaMap, Collections.emptyMap(), collection)); } - DocCollection col = new DocCollection(collection, sliceMap, Collections.emptyMap(), DocRouter.DEFAULT); + @SuppressWarnings({"unchecked"}) + DocCollection col = new DocCollection(collection, sliceMap, Collections.singletonMap(ZkStateReader.CONFIGNAME_PROP, ConfigSetsHandler.DEFAULT_CONFIGSET_NAME), DocRouter.DEFAULT); collToState.put(collection, col); } // reverse map 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 aeb85ba092d5..eabc4a73423d 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 @@ -217,6 +217,7 @@ public String toString() { /** * Create a ClusterState from Json. + * This method doesn't support legacy configName location and thus don't call it where that's important * * @param bytes a byte array of a Json representation of a mapping from collection name to the Json representation of a * {@link DocCollection} as written by {@link #write(JSONWriter)}. It can represent @@ -233,6 +234,50 @@ public static ClusterState createFromJson(int version, byte[] bytes, Set return createFromCollectionMap(version, stateMap, liveNodes); } + /** + * Create a ClusterState from Json. + * This method supports legacy configName location + * + * @param bytes a byte array of a Json representation of a mapping from collection name to the Json representation of a + * {@link DocCollection} as written by {@link #write(JSONWriter)}. It can represent + * one or more collections. + * @param liveNodes list of live nodes + * @param coll collection name + * @param zkClient ZK client + * @return the ClusterState + */ + @SuppressWarnings({"unchecked"}) + @Deprecated + public static ClusterState createFromJsonSupportingLegacyConfigName(int version, byte[] bytes, Set liveNodes, String coll, SolrZkClient zkClient) { + if (bytes == null || bytes.length == 0) { + return new ClusterState(liveNodes, Collections.emptyMap()); + } + Map stateMap = (Map) Utils.fromJSON(bytes); + Map props = (Map) stateMap.get(coll); + if (props != null) { + if (!props.containsKey(ZkStateReader.CONFIGNAME_PROP)) { + try { + // read configName from collections/collection node + String path = ZkStateReader.COLLECTIONS_ZKNODE + "/" + coll; + byte[] data = zkClient.getData(path, null, null, true); + if (data != null && data.length > 0) { + ZkNodeProps configProp = ZkNodeProps.load(data); + String configName = configProp.getStr(ZkStateReader.CONFIGNAME_PROP); + if (configName != null) { + props.put(ZkStateReader.CONFIGNAME_PROP, configName); + stateMap.put(coll, props); + } else { + log.warn("configName is null, not found on {}", path); + } + } + } catch (KeeperException | InterruptedException e) { + // do nothing + } + } + } + return createFromCollectionMap(version, stateMap, liveNodes); + } + public static ClusterState createFromCollectionMap(int version, Map stateMap, Set liveNodes) { Map collections = new LinkedHashMap<>(stateMap.size()); for (Entry entry : stateMap.entrySet()) { diff --git a/solr/solrj/src/java/org/apache/solr/common/cloud/DocCollection.java b/solr/solrj/src/java/org/apache/solr/common/cloud/DocCollection.java index c79168a306ec..e0e013c8502a 100644 --- a/solr/solrj/src/java/org/apache/solr/common/cloud/DocCollection.java +++ b/solr/solrj/src/java/org/apache/solr/common/cloud/DocCollection.java @@ -39,6 +39,7 @@ import static org.apache.solr.common.cloud.ZkStateReader.READ_ONLY; import static org.apache.solr.common.cloud.ZkStateReader.REPLICATION_FACTOR; import static org.apache.solr.common.cloud.ZkStateReader.TLOG_REPLICAS; +import static org.apache.solr.common.cloud.ZkStateReader.CONFIGNAME_PROP; import static org.apache.solr.common.util.Utils.toJSONString; /** @@ -55,6 +56,7 @@ public class DocCollection extends ZkNodeProps implements Iterable { private final int znodeVersion; private final String name; + private final String configName; private final Map slices; private final Map activeSlices; private final Slice[] activeSlicesArr; @@ -84,11 +86,11 @@ public DocCollection(String name, Map slices, Map * @param zkVersion The version of the Collection node in Zookeeper (used for conditional updates). */ public DocCollection(String name, Map slices, Map props, DocRouter router, int zkVersion) { - super(props==null ? props = new HashMap<>() : props); + super(props); // -1 means any version in ZK CAS, so we choose Integer.MAX_VALUE instead to avoid accidental overwrites this.znodeVersion = zkVersion == -1 ? Integer.MAX_VALUE : zkVersion; this.name = name; - + this.configName = (String) props.get(CONFIGNAME_PROP); this.slices = slices; this.activeSlices = new HashMap<>(); this.nodeNameLeaderReplicas = new HashMap<>(); @@ -206,6 +208,14 @@ public String getName() { return name; } + /** + * Return non-null config name + */ + public String getConfigName() { + assert configName != null; + return configName; + } + public Slice getSlice(String sliceName) { return slices.get(sliceName); } 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 0cfbbab8d697..3401ee7fe68e 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 @@ -231,42 +231,6 @@ public boolean canBeRemoved() { PLACEMENT_PLUGIN ); - /** - * Returns config set name for collection. - * TODO move to DocCollection (state.json). - * - * @param collection to return config set name for - */ - public String readConfigName(String collection) throws KeeperException { - - String configName = null; - - String path = COLLECTIONS_ZKNODE + "/" + collection; - log.debug("Loading collection config from: [{}]", path); - - try { - byte[] data = zkClient.getData(path, null, null, true); - if (data == null) { - log.warn("No config data found at path {}.", path); - throw new KeeperException.NoNodeException("No config data found at path: " + path); - } - - ZkNodeProps props = ZkNodeProps.load(data); - configName = props.getStr(CONFIGNAME_PROP); - - if (configName == null) { - log.warn("No config data found at path{}. ", path); - throw new KeeperException.NoNodeException("No config data found at path: " + path); - } - } catch (InterruptedException e) { - SolrZkClient.checkInterrupted(e); - log.warn("Thread interrupted when loading config name for collection {}", collection); - throw new SolrException(ErrorCode.SERVER_ERROR, "Thread interrupted when loading config name for collection " + collection, e); - } - - return configName; - } - private final SolrZkClient zkClient; @@ -1464,7 +1428,11 @@ private DocCollection fetchCollectionState(String coll, Watcher watcher) throws try { Stat stat = new Stat(); byte[] data = zkClient.getData(collectionPath, watcher, stat, true); - ClusterState state = ClusterState.createFromJson(stat.getVersion(), data, Collections.emptySet()); + + // This factory method can detect a missing configName and supply it by reading it from the old ZK location. + // TODO in Solr 10 remove that factory method + ClusterState state = ClusterState.createFromJsonSupportingLegacyConfigName(stat.getVersion(), data, Collections.emptySet(), coll, zkClient); + ClusterState.CollectionRef collectionRef = state.getCollectionStates().get(coll); return collectionRef == null ? null : collectionRef.get(); } catch (KeeperException.NoNodeException e) { diff --git a/solr/test-framework/src/java/org/apache/solr/cloud/api/collections/AbstractCloudBackupRestoreTestCase.java b/solr/test-framework/src/java/org/apache/solr/cloud/api/collections/AbstractCloudBackupRestoreTestCase.java index 4cd63a8cb443..357531aa1adb 100644 --- a/solr/test-framework/src/java/org/apache/solr/cloud/api/collections/AbstractCloudBackupRestoreTestCase.java +++ b/solr/test-framework/src/java/org/apache/solr/cloud/api/collections/AbstractCloudBackupRestoreTestCase.java @@ -369,8 +369,7 @@ private void testBackupAndRestore(String collectionName, int backupReplFactor) t assertEquals(origShardToDocCount, getShardToDocCountMap(client, restoreCollection)); } - assertEquals(sameConfig ? "conf1" : "customConfigName", - cluster.getSolrClient().getZkStateReader().readConfigName(restoreCollectionName)); + assertEquals(sameConfig ? "conf1" : "customConfigName", restoreCollection.getConfigName()); Map numReplicasByNodeName = new HashMap<>(); restoreCollection.getReplicas().forEach(x -> {