diff --git a/changelog/unreleased/SOLR-18166-remove-linkconfig.yml b/changelog/unreleased/SOLR-18166-remove-linkconfig.yml new file mode 100644 index 000000000000..c600972f6c92 --- /dev/null +++ b/changelog/unreleased/SOLR-18166-remove-linkconfig.yml @@ -0,0 +1,31 @@ +# (DELETE ALL COMMENTS UP HERE AFTER FILLING THIS IN + +# See https://github.com/apache/solr/blob/main/dev-docs/changelog.adoc + +# If the change is minor, don't bother adding a changelog entry. +# For `other` type entries, the threshold to bother with a changelog entry should be even higher. + +# title: +# * The audience is end-users and administrators, not committers. +# * Be short and focused on the user impact. Multiple sentences is fine! +# * For technical/geeky details, prefer the commit message instead of changelog. +# * Reference JIRA issues like `SOLR-12345`, or if no JIRA but have a GitHub PR then `PR#12345`. + +# type: +# `added` for new features/improvements, opt-in by the user typically documented in the ref guide +# `changed` for improvements; not opt-in +# `fixed` for improvements that are deemed to have fixed buggy behavior +# `deprecated` for marking things deprecated +# `removed` for code removed +# `dependency_update` for updates to dependencies +# `other` for anything else, like large/significant refactorings, build changes, +# test infrastructure, or documentation. +# Most such changes are too small/minor to bother with a changelog entry. + +title: Remove outmoded and misleading 'bin/solr zk linkconfig' tool. +type: removed +authors: + - name: Jason Gerlowski +links: + - name: SOLR-18166 + url: https://issues.apache.org/jira/browse/SOLR-18166 diff --git a/solr/core/src/java/org/apache/solr/cli/LinkConfigTool.java b/solr/core/src/java/org/apache/solr/cli/LinkConfigTool.java deleted file mode 100644 index cb46c6e81933..000000000000 --- a/solr/core/src/java/org/apache/solr/cli/LinkConfigTool.java +++ /dev/null @@ -1,87 +0,0 @@ -/* - * Licensed to the Apache Software Foundation (ASF) under one or more - * contributor license agreements. See the NOTICE file distributed with - * this work for additional information regarding copyright ownership. - * The ASF licenses this file to You under the Apache License, Version 2.0 - * (the "License"); you may not use this file except in compliance with - * the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package org.apache.solr.cli; - -import java.util.concurrent.TimeUnit; -import org.apache.commons.cli.CommandLine; -import org.apache.commons.cli.Option; -import org.apache.commons.cli.Options; -import org.apache.solr.client.solrj.impl.SolrZkClientTimeout; -import org.apache.solr.cloud.ZkController; -import org.apache.solr.common.cloud.SolrZkClient; - -/** Supports linking a configset to a collection */ -public class LinkConfigTool extends ToolBase { - - private static final Option COLLECTION_NAME_OPTION = - Option.builder("c") - .longOpt("name") - .hasArg() - .argName("NAME") - .required() - .desc("Name of the collection to link.") - .get(); - - private static final Option CONF_NAME_OPTION = - Option.builder("n") - .longOpt("conf-name") - .hasArg() - .argName("NAME") - .required() - .desc("Configset name in ZooKeeper.") - .get(); - - public LinkConfigTool(ToolRuntime runtime) { - super(runtime); - } - - @Override - public String getName() { - return "linkconfig"; - } - - @Override - public String getUsage() { - return "bin/solr zk linkconfig -c -n [-z ]"; - } - - @Override - public Options getOptions() { - return super.getOptions() - .addOption(COLLECTION_NAME_OPTION) - .addOption(CONF_NAME_OPTION) - .addOption(CommonCLIOptions.ZK_HOST_OPTION); - } - - @Override - public void runImpl(CommandLine cli) throws Exception { - - String collection = cli.getOptionValue(COLLECTION_NAME_OPTION); - String confName = cli.getOptionValue(CONF_NAME_OPTION); - String zkHost = CLIUtils.getZkHost(cli); - - try (SolrZkClient zkClient = - new SolrZkClient.Builder() - .withUrl(zkHost) - .withTimeout(SolrZkClientTimeout.DEFAULT_ZK_CLIENT_TIMEOUT, TimeUnit.MILLISECONDS) - .build()) { - - ZkController.linkConfSet(zkClient, collection, confName); - } - } -} diff --git a/solr/core/src/java/org/apache/solr/cli/SolrCLI.java b/solr/core/src/java/org/apache/solr/cli/SolrCLI.java index 80c7c1a02894..9f34c32cb371 100755 --- a/solr/core/src/java/org/apache/solr/cli/SolrCLI.java +++ b/solr/core/src/java/org/apache/solr/cli/SolrCLI.java @@ -73,8 +73,7 @@ public static void main(String[] args) throws Exception { // select the version tool to be run args = new String[] {"version"}; } - if (Arrays.asList( - "upconfig", "downconfig", "cp", "rm", "mv", "ls", "mkroot", "linkconfig", "updateacls") + if (Arrays.asList("upconfig", "downconfig", "cp", "rm", "mv", "ls", "mkroot", "updateacls") .contains(args[0])) { // remap our arguments to invoke the zk short tool help args = new String[] {"zk-tool-help", "--print-zk-subcommand-usage", args[0]}; @@ -194,7 +193,6 @@ private static Tool newTool(String toolType, ToolRuntime runtime) throws Excepti else if ("ls".equals(toolType)) return new ZkLsTool(runtime); else if ("cluster".equals(toolType)) return new ClusterTool(runtime); else if ("updateacls".equals(toolType)) return new UpdateACLTool(runtime); - else if ("linkconfig".equals(toolType)) return new LinkConfigTool(runtime); else if ("mkroot".equals(toolType)) return new ZkMkrootTool(runtime); else if ("assert".equals(toolType)) return new AssertTool(runtime); else if ("auth".equals(toolType)) return new AuthTool(runtime); diff --git a/solr/core/src/java/org/apache/solr/cli/ZkToolHelp.java b/solr/core/src/java/org/apache/solr/cli/ZkToolHelp.java index 84f0040ae23e..9251484a859f 100644 --- a/solr/core/src/java/org/apache/solr/cli/ZkToolHelp.java +++ b/solr/core/src/java/org/apache/solr/cli/ZkToolHelp.java @@ -71,7 +71,6 @@ public void runImpl(CommandLine cli) throws Exception { print(new ConfigSetUploadTool(runtime).getUsage()); print(new ConfigSetDownloadTool(runtime).getUsage()); print(new ZkMkrootTool(runtime).getUsage()); - print(new LinkConfigTool(runtime).getUsage()); print(new UpdateACLTool(runtime).getUsage()); print(""); print( 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 6d686e13d79a..018d396a80dd 100644 --- a/solr/core/src/java/org/apache/solr/cloud/DistributedClusterStateUpdater.java +++ b/solr/core/src/java/org/apache/solr/cloud/DistributedClusterStateUpdater.java @@ -30,7 +30,6 @@ import java.util.Optional; import java.util.Set; import org.apache.solr.client.solrj.cloud.SolrCloudManager; -import org.apache.solr.client.solrj.impl.ZkClientClusterStateProvider; import org.apache.solr.cloud.overseer.ClusterStateMutator; import org.apache.solr.cloud.overseer.CollectionMutator; import org.apache.solr.cloud.overseer.NodeMutator; @@ -573,26 +572,20 @@ private void doStateDotJsonCasUpdate(ClusterState updatedState) * enough... (we have to deal anyway with failures of conditional updates so trying to use non * fresh data is ok, a second attempt will be made) */ + @SuppressWarnings("unchecked") private ClusterState fetchStateForCollection() throws KeeperException, InterruptedException { String collectionStatePath = DocCollection.getCollectionPath(updater.getCollectionName()); Stat stat = new Stat(); byte[] data = zkStateReader.getZkClient().getData(collectionStatePath, null, stat); - - // 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 = - ZkClientClusterStateProvider.createFromJsonSupportingLegacyConfigName( - stat.getVersion(), - data, - Set.of(), - updater.getCollectionName(), - zkStateReader.getZkClient(), - Instant.ofEpochMilli(stat.getCtime())); - - return clusterState; + Map stateMap = (Map) Utils.fromJSON(data); + + return ClusterState.createFromCollectionMap( + stat.getVersion(), + stateMap, + Set.of(), + Instant.ofEpochMilli(stat.getCtime()), + PerReplicaStatesOps.getZkClientPrsSupplier( + zkStateReader.getZkClient(), collectionStatePath)); } } diff --git a/solr/core/src/java/org/apache/solr/cloud/ZkController.java b/solr/core/src/java/org/apache/solr/cloud/ZkController.java index 06bfdf24df88..4b574750ede8 100644 --- a/solr/core/src/java/org/apache/solr/cloud/ZkController.java +++ b/solr/core/src/java/org/apache/solr/cloud/ZkController.java @@ -2378,46 +2378,6 @@ private ZkCoreNodeProps waitForLeaderToSeeDownState( return leaderProps; } - public static void linkConfSet(SolrZkClient zkClient, String collection, String confSetName) - throws KeeperException, InterruptedException { - String path = ZkStateReader.COLLECTIONS_ZKNODE + "/" + collection; - log.debug("Load collection config from:{}", path); - byte[] data; - try { - data = zkClient.getData(path, null, null); - } catch (NoNodeException e) { - // if there is no node, we will try and create it - // first try to make in case we are pre-configuring - ZkNodeProps props = new ZkNodeProps(CONFIGNAME_PROP, confSetName); - try { - - zkClient.makePath(path, Utils.toJSON(props), CreateMode.PERSISTENT, null, true); - } catch (KeeperException e2) { - // it's okay if the node already exists - if (e2.code() != KeeperException.Code.NODEEXISTS) { - throw e; - } - // if we fail creating, setdata - // TODO: we should consider using version - zkClient.setData(path, Utils.toJSON(props)); - } - return; - } - // we found existing data, let's update it - ZkNodeProps props = null; - if (data != null) { - props = ZkNodeProps.load(data); - Map newProps = new HashMap<>(props.getProperties()); - newProps.put(CONFIGNAME_PROP, confSetName); - props = new ZkNodeProps(newProps); - } else { - props = new ZkNodeProps(CONFIGNAME_PROP, confSetName); - } - - // TODO: we should consider using version - zkClient.setData(path, Utils.toJSON(props)); - } - public ZkDistributedQueue getOverseerJobQueue() { if (distributedClusterStateUpdater.isDistributedStateUpdate()) { throw new IllegalStateException( diff --git a/solr/core/src/test/org/apache/solr/cli/ZkSubcommandsTest.java b/solr/core/src/test/org/apache/solr/cli/ZkSubcommandsTest.java index 5ddd4f4048be..d5aa1a72d5ae 100644 --- a/solr/core/src/test/org/apache/solr/cli/ZkSubcommandsTest.java +++ b/solr/core/src/test/org/apache/solr/cli/ZkSubcommandsTest.java @@ -36,8 +36,6 @@ import org.apache.solr.common.cloud.DigestZkCredentialsProvider; import org.apache.solr.common.cloud.SolrZkClient; import org.apache.solr.common.cloud.VMParamsZkCredentialsInjector; -import org.apache.solr.common.cloud.ZkNodeProps; -import org.apache.solr.common.cloud.ZkStateReader; import org.apache.solr.common.util.ZLibCompressor; import org.apache.solr.core.ConfigSetService; import org.apache.solr.util.ExternalPaths; @@ -303,7 +301,7 @@ public void testLs() throws Exception { } @Test - public void testUpConfigLinkConfigClearZk() throws Exception { + public void testUpConfigDownConfigClearZk() throws Exception { Path tmpDir = createTempDir(); // test upconfig @@ -333,26 +331,6 @@ public void testUpConfigLinkConfigClearZk() throws Exception { "Verify that all local files are uploaded to ZK", filesStream.count(), zkFiles.size()); } - // test linkconfig - args = - new String[] { - "linkconfig", - "--conf-name", - confsetname, - "-c", - "collection1", - "-z", - zkServer.getZkAddress() - }; - - assertEquals(0, CLITestHelper.runTool(args, LinkConfigTool.class)); - - ZkNodeProps collectionProps = - ZkNodeProps.load( - zkClient.getData(ZkStateReader.COLLECTIONS_ZKNODE + "/collection1", null, null)); - assertTrue(collectionProps.containsKey("configName")); - assertEquals(confsetname, collectionProps.getStr("configName")); - // test down config Path destDir = FilterPath.unwrap( diff --git a/solr/solrj-zookeeper/src/java/org/apache/solr/client/solrj/impl/ZkClientClusterStateProvider.java b/solr/solrj-zookeeper/src/java/org/apache/solr/client/solrj/impl/ZkClientClusterStateProvider.java index fd3525b564b0..0de2184ad5f4 100644 --- a/solr/solrj-zookeeper/src/java/org/apache/solr/client/solrj/impl/ZkClientClusterStateProvider.java +++ b/solr/solrj-zookeeper/src/java/org/apache/solr/client/solrj/impl/ZkClientClusterStateProvider.java @@ -19,7 +19,6 @@ import java.io.IOException; import java.lang.invoke.MethodHandles; -import java.time.Instant; import java.util.Collection; import java.util.List; import java.util.Map; @@ -28,16 +27,10 @@ import org.apache.solr.common.AlreadyClosedException; import org.apache.solr.common.SolrException; import org.apache.solr.common.cloud.ClusterState; -import org.apache.solr.common.cloud.DocCollection; -import org.apache.solr.common.cloud.PerReplicaStatesOps; -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.cloud.ZooKeeperException; import org.apache.solr.common.util.EnvUtils; -import org.apache.solr.common.util.Utils; import org.apache.zookeeper.KeeperException; -import org.noggit.JSONWriter; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -85,62 +78,6 @@ public ZkClientClusterStateProvider(String zkHost) { this.canUseZkACLs = true; } - /** - * 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 - * ClusterState#write(JSONWriter)}. It can represent one or more collections. - * @param liveNodes list of live nodes - * @param coll collection name - * @param zkClient ZK client - * @param createTime creation time of the data/bytes - * @return the ClusterState - */ - @SuppressWarnings({"unchecked"}) - @Deprecated - public static ClusterState createFromJsonSupportingLegacyConfigName( - int version, - byte[] bytes, - Set liveNodes, - String coll, - SolrZkClient zkClient, - Instant createTime) { - if (bytes == null || bytes.length == 0) { - return new ClusterState(liveNodes, Map.of()); - } - 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); - 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 ClusterState.createFromCollectionMap( - version, - stateMap, - liveNodes, - createTime, - PerReplicaStatesOps.getZkClientPrsSupplier( - zkClient, DocCollection.getCollectionPath(coll))); - } - @Override public ClusterState.CollectionRef getState(String collection) { ClusterState clusterState = getZkStateReader().getClusterState(); diff --git a/solr/solrj-zookeeper/src/java/org/apache/solr/common/cloud/ZkStateReader.java b/solr/solrj-zookeeper/src/java/org/apache/solr/common/cloud/ZkStateReader.java index 88033012b9f9..763a3326b5a7 100644 --- a/solr/solrj-zookeeper/src/java/org/apache/solr/common/cloud/ZkStateReader.java +++ b/solr/solrj-zookeeper/src/java/org/apache/solr/common/cloud/ZkStateReader.java @@ -1438,6 +1438,7 @@ public DocCollection getCollectionLive(String coll) { } } + @SuppressWarnings("unchecked") private DocCollection fetchCollectionState(String coll, Watcher watcher) throws KeeperException, InterruptedException { String collectionPath = DocCollection.getCollectionPath(coll); @@ -1445,19 +1446,16 @@ private DocCollection fetchCollectionState(String coll, Watcher watcher) try { Stat stat = new Stat(); byte[] data = zkClient.getData(collectionPath, watcher, stat); + Map stateMap = (Map) Utils.fromJSON(data); - // 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 = - ZkClientClusterStateProvider.createFromJsonSupportingLegacyConfigName( + final var state = + ClusterState.createFromCollectionMap( stat.getVersion(), - data, - Set.of(), - coll, - zkClient, - Instant.ofEpochMilli(stat.getCtime())); - + stateMap, + liveNodes, + Instant.ofEpochMilli(stat.getCtime()), + PerReplicaStatesOps.getZkClientPrsSupplier( + zkClient, DocCollection.getCollectionPath(coll))); return state.getCollectionOrNull(coll); } catch (KeeperException.NoNodeException e) { if (watcher != null) {