dsmiley commented on a change in pull request #23:
URL: https://github.com/apache/solr/pull/23#discussion_r598745515
##########
File path: solr/core/src/java/org/apache/solr/core/backup/BackupManager.java
##########
@@ -253,7 +253,7 @@ public void downloadConfigDir(String configName) throws
IOException {
repository.createDirectory(getZkStateDir(CONFIG_STATE_DIR));
repository.createDirectory(dest);
- downloadFromZK(zkStateReader.getZkClient(), ZkConfigManager.CONFIGS_ZKNODE
+ "/" + configName, dest);
+ downloadFromZK(zkStateReader.getZkClient(),
ZkConfigSetService.CONFIGS_ZKNODE + "/" + configName, dest);
Review comment:
BackupManager should be using configSetService completely to manage
configSets, not ZK
##########
File path:
solr/core/src/java/org/apache/solr/cloud/api/collections/CollectionHandlingUtils.java
##########
@@ -301,7 +302,7 @@ static void deleteBackup(BackupRepository repository, URI
backupPath, int maxNum
}
static void validateConfigOrThrowSolrException(SolrCloudManager
cloudManager, String configName) throws IOException, KeeperException,
InterruptedException {
- boolean isValid =
cloudManager.getDistribStateManager().hasData(ZkConfigManager.CONFIGS_ZKNODE +
"/" + configName);
+ boolean isValid =
cloudManager.getDistribStateManager().hasData(ZkConfigSetService.CONFIGS_ZKNODE
+ "/" + configName);
Review comment:
bad dependency on where configSets are located; use configSetService.
Maybe SolrCloudManager should have CSS?
##########
File path:
solr/core/src/java/org/apache/solr/cloud/OverseerCollectionConfigSetProcessor.java
##########
@@ -88,9 +88,9 @@ private static OverseerMessageHandlerSelector
getOverseerMessageHandlerSelector(
Overseer overseer,
OverseerNodePrioritizer overseerNodePrioritizer) {
final OverseerCollectionMessageHandler collMessageHandler = new
OverseerCollectionMessageHandler(
- zkStateReader, myId, shardHandlerFactory, adminPath, stats, overseer,
overseerNodePrioritizer);
+ zkStateReader, myId, shardHandlerFactory, adminPath, stats, overseer,
overseerNodePrioritizer, overseer.getCoreContainer());
final OverseerConfigSetMessageHandler configMessageHandler = new
OverseerConfigSetMessageHandler(
- zkStateReader);
+ zkStateReader, overseer.getCoreContainer());
Review comment:
Can't we just pass ConfigSetService?
##########
File path:
solr/core/src/java/org/apache/solr/cloud/OverseerConfigSetMessageHandler.java
##########
@@ -286,7 +288,7 @@ private void mergeOldProperties(Map<String, Object>
newProps, @SuppressWarnings(
}
private String getPropertyPath(String configName, String propertyPath) {
- return ZkConfigManager.CONFIGS_ZKNODE + "/" + configName + "/" +
propertyPath;
+ return ZkConfigSetService.CONFIGS_ZKNODE + "/" + configName + "/" +
propertyPath;
Review comment:
Suspicious. getConfigSetProperties is assuming ZK. Review this whole
class to ensure ZK references are reconsidered.
##########
File path: solr/core/src/java/org/apache/solr/cloud/ZkConfigSetService.java
##########
@@ -0,0 +1,319 @@
+/*
+ * 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.cloud;
+
+import java.io.File;
+import java.io.IOException;
+import java.lang.invoke.MethodHandles;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.util.Collections;
+import java.util.List;
+import java.util.Locale;
+import java.util.Set;
+import java.util.regex.Pattern;
+
+import org.apache.solr.client.solrj.cloud.SolrCloudManager;
+import org.apache.solr.cloud.api.collections.CreateCollectionCmd;
+import org.apache.solr.common.SolrException;
+import org.apache.solr.common.cloud.SolrZkClient;
+import org.apache.solr.common.cloud.ZkStateReader;
+import org.apache.solr.common.cloud.ZooKeeperException;
+import org.apache.solr.common.util.NamedList;
+import org.apache.solr.core.ConfigSetProperties;
+import org.apache.solr.core.ConfigSetService;
+import org.apache.solr.core.CoreContainer;
+import org.apache.solr.core.CoreDescriptor;
+import org.apache.solr.core.SolrConfig;
+import org.apache.solr.core.SolrResourceLoader;
+import org.apache.solr.handler.admin.ConfigSetsHandler;
+import org.apache.solr.servlet.SolrDispatchFilter;
+import org.apache.zookeeper.KeeperException;
+import org.apache.zookeeper.data.Stat;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * SolrCloud Zookeeper ConfigSetService impl.
+ */
+public class ZkConfigSetService extends ConfigSetService {
+ private static final Logger log =
LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
+ private final ZkController zkController;
+ private final SolrZkClient zkClient;
+ /** ZkNode where named configs are stored */
+ public static final String CONFIGS_ZKNODE = "/configs";
+ public static final String UPLOAD_FILENAME_EXCLUDE_REGEX = "^\\..*$";
+ public static final Pattern UPLOAD_FILENAME_EXCLUDE_PATTERN =
Pattern.compile(UPLOAD_FILENAME_EXCLUDE_REGEX);
+
+ public ZkConfigSetService(CoreContainer cc) {
+ super(cc.getResourceLoader(), cc.getConfig().hasSchemaCache());
+ this.zkController = cc.getZkController();
+ this.zkClient = cc.getZkController().getZkClient();
+ try {
+ bootstrapDefaultConfigSet();
+ } catch (IOException e) {
+ log.error("Error in bootstrapping default config");
+ }
+ }
+
+ /** This is for ZkCLI and some tests */
+ public ZkConfigSetService(SolrZkClient zkClient) {
+ super(null, false);
+ this.zkController = null;
+ this.zkClient = zkClient;
+ }
+
+ @Override
+ public SolrResourceLoader createCoreResourceLoader(CoreDescriptor cd) {
+ final String colName = cd.getCollectionName();
+
+ // For back compat with cores that can create collections without the
collections API
+ try {
+ if (!zkClient.exists(ZkStateReader.COLLECTIONS_ZKNODE + "/" + colName,
true)) {
+ // TODO remove this functionality or maybe move to a CLI mechanism
+ log.warn("Auto-creating collection (in ZK) from core descriptor (on
disk). This feature may go away!");
+
CreateCollectionCmd.createCollectionZkNode(zkController.getSolrCloudManager().getDistribStateManager(),
colName, cd.getCloudDescriptor().getParams());
+ }
+ } catch (InterruptedException e) {
+ Thread.currentThread().interrupt();
+ throw new ZooKeeperException(SolrException.ErrorCode.SERVER_ERROR,
"Interrupted auto-creating collection", e);
+ } catch (KeeperException e) {
+ throw new ZooKeeperException(SolrException.ErrorCode.SERVER_ERROR,
"Failure auto-creating collection", e);
+ }
+
+ // 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);
+ 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);
+ }
+
+ @Override
+ @SuppressWarnings({"rawtypes"})
+ protected NamedList loadConfigSetFlags(CoreDescriptor cd, SolrResourceLoader
loader) {
+ try {
+ return ConfigSetProperties.readFromResourceLoader(loader, ".");
+ } catch (Exception ex) {
+ log.debug("No configSet flags", ex);
+ return null;
+ }
+ }
+
+ @Override
+ protected Long getCurrentSchemaModificationVersion(String configSet,
SolrConfig solrConfig, String schemaFile) {
+ String zkPath = CONFIGS_ZKNODE + "/" + configSet + "/" + schemaFile;
+ Stat stat;
+ try {
+ stat = zkClient.exists(zkPath, null, true);
+ } catch (KeeperException e) {
+ log.warn("Unexpected exception when getting modification time of {}",
zkPath, e);
+ return null; // debatable; we'll see an error soon if there's a real
problem
+ } catch (InterruptedException e) {
+ Thread.currentThread().interrupt();
+ throw new SolrException(SolrException.ErrorCode.SERVER_ERROR, e);
+ }
+ if (stat == null) { // not found
+ return null;
+ }
+ return (long) stat.getVersion();
+ }
+
+ @Override
+ public String configSetName(CoreDescriptor cd) {
+ return "configset " + cd.getConfigSet();
+ }
+
+ @Override
+ public boolean checkConfigExists(String configName) throws IOException {
+ try {
+ return zkClient.exists(CONFIGS_ZKNODE + "/" + configName, true);
+ } catch (KeeperException | InterruptedException e) {
+ throw new IOException("Error checking whether config exists",
+ SolrZkClient.checkInterrupted(e));
+ }
+ }
+
+ @Override
+ public void deleteConfigDir(String configName) throws IOException {
+ try {
+ zkClient.clean(CONFIGS_ZKNODE + "/" + configName);
+ } catch (KeeperException | InterruptedException e) {
+ throw new IOException("Error checking whether config exists",
+ SolrZkClient.checkInterrupted(e));
+ }
+ }
+
+ @Override
+ public void copyConfigDir(String fromConfig, String toConfig) throws
IOException {
+ copyConfigDir(fromConfig, toConfig, null);
+ }
+
+ @Override
+ public void copyConfigDir(String fromConfig, String toConfig, Set<String>
copiedToZkPaths) throws IOException {
+ String fromConfigPath = CONFIGS_ZKNODE + "/" + fromConfig;
+ String toConfigPath = CONFIGS_ZKNODE + "/" + toConfig;
+ try {
+ copyData(copiedToZkPaths, fromConfigPath, toConfigPath);
+ } catch (KeeperException | InterruptedException e) {
+ throw new IOException("Error config " + fromConfig + " to " + toConfig,
+ SolrZkClient.checkInterrupted(e));
+ }
+ copyConfigDirFromZk(fromConfigPath, toConfigPath, copiedToZkPaths);
+ }
+
+ @Override
+ public void uploadConfigDir(Path dir, String configName) throws IOException {
+ zkClient.uploadToZK(dir, CONFIGS_ZKNODE + "/" + configName,
ZkConfigSetService.UPLOAD_FILENAME_EXCLUDE_PATTERN);
+ }
+
+ @Override
+ public void downloadConfigDir(String configName, Path dir) throws
IOException {
+ zkClient.downloadFromZK(CONFIGS_ZKNODE + "/" + configName, dir);
+ }
+
+ @Override
+ public List<String> listConfigs() throws IOException {
+ try {
+ return zkClient.getChildren(CONFIGS_ZKNODE, null, true);
+ } catch (KeeperException.NoNodeException e) {
+ return Collections.emptyList();
+ } catch (KeeperException | InterruptedException e) {
+ throw new IOException("Error listing configs",
SolrZkClient.checkInterrupted(e));
+ }
+ }
+
+ private void bootstrapDefaultConfigSet() throws IOException {
Review comment:
Doesn't seem specific to ZK.
##########
File path: solr/core/src/java/org/apache/solr/core/ConfigSetService.java
##########
@@ -211,6 +215,73 @@ protected NamedList loadConfigSetFlags(CoreDescriptor cd,
SolrResourceLoader loa
*/
public abstract String configSetName(CoreDescriptor cd);
+ /**
+ * Check whether a config exists in Zookeeper
+ *
+ * @param configName the config to check existance on
+ * @return whether the config exists or not
+ * @throws IOException if an I/O error occurs
+ */
+ public abstract Boolean configExists(String configName) throws IOException;
+
+ /**
+ * Delete a config in ZooKeeper
+ *
+ * @param configName the config to delete
+ * @throws IOException if an I/O error occurs
+ */
+ public abstract void deleteConfigDir(String configName) throws IOException;
Review comment:
Yes; do that and to the other methods with "Dir"
##########
File path:
solr/core/src/java/org/apache/solr/handler/admin/ConfigSetsHandler.java
##########
@@ -167,7 +167,7 @@ private void handleConfigUploadRequest(SolrQueryRequest
req, SolrQueryResponse r
}
SolrZkClient zkClient = coreContainer.getZkController().getZkClient();
- String configPathInZk = ZkConfigManager.CONFIGS_ZKNODE + "/" +
configSetName;
+ String configPathInZk = ZkConfigSetService.CONFIGS_ZKNODE + "/" +
configSetName;
Review comment:
Should use ConfigSetService abstraction
##########
File path:
solr/core/src/java/org/apache/solr/core/FileSystemConfigSetService.java
##########
@@ -0,0 +1,119 @@
+/*
+ * 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.core;
+
+import java.io.FileNotFoundException;
+import java.io.IOException;
+import java.lang.invoke.MethodHandles;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.util.List;
+import java.util.Set;
+import java.util.stream.Collectors;
+
+import org.apache.solr.common.SolrException;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * The Solr standalone version of File System ConfigSetService.
+ * <p>
+ * Loads a ConfigSet defined by the core's configSet property,
+ * looking for a directory named for the configSet property value underneath
+ * a base directory. If no configSet property is set, loads the ConfigSet
+ * instead from the core's instance directory.
+ */
+public class FileSystemConfigSetService extends ConfigSetService {
+ private static final Logger log =
LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
+ private final Path configSetBase;
+
+ public FileSystemConfigSetService(CoreContainer cc) {
+ super(cc.getResourceLoader(), cc.getConfig().hasSchemaCache());
+ this.configSetBase = cc.getConfig().getConfigSetBaseDirectory();
+ }
+
+ @Override
+ public SolrResourceLoader createCoreResourceLoader(CoreDescriptor cd) {
+ Path instanceDir = locateInstanceDir(cd);
+ SolrResourceLoader solrResourceLoader = new
SolrResourceLoader(instanceDir, parentLoader.getClassLoader());
+ return solrResourceLoader;
+ }
+
+ @Override
+ public String configSetName(CoreDescriptor cd) {
+ return (cd.getConfigSet() == null ? "instancedir " : "configset ") +
locateInstanceDir(cd);
+ }
+
+ @Override
+ public void deleteConfigDir(String configName) throws IOException {
+ throw new UnsupportedOperationException("Only supported with
ZkConfigSetService");
Review comment:
Let's not mention other ConfigSetService impls. I dunno if UOE can have
no arg at all; we don't need to say anything special.
##########
File path: solr/core/src/java/org/apache/solr/core/ConfigSetService.java
##########
@@ -216,72 +237,76 @@ protected NamedList loadConfigSetFlags(CoreDescriptor cd,
SolrResourceLoader loa
*/
public abstract String configSetName(CoreDescriptor cd);
- public interface ConfigResource {
-
- ConfigNode get() throws Exception;
-
+ /**
+ * Check whether a config exists
+ *
+ * @param configName the config to check existance on
+ * @return whether the config exists or not
+ * @throws IOException if an I/O error occurs
+ */
+ public boolean checkConfigExists(String configName) throws IOException {
+ return listConfigs().contains(configName);
}
- public static ConfigNode getParsedSchema(InputStream is, SolrResourceLoader
loader, String name) throws IOException, SAXException,
ParserConfigurationException {
- XmlConfigFile schemaConf = null;
- InputSource inputSource = new InputSource(is);
-
inputSource.setSystemId(SystemIdResolver.createSystemIdFromResourceName(name));
- schemaConf = new XmlConfigFile(loader, SCHEMA, inputSource, SLASH + SCHEMA
+ SLASH, null);
- return new DataConfigNode(new
DOMConfigNode(schemaConf.getDocument().getDocumentElement()));
- }
+ /**
+ * Delete a config
+ *
+ * @param configName the config to delete
+ * @throws IOException if an I/O error occurs
+ */
+ public abstract void deleteConfigDir(String configName) throws IOException;
+
+ /**
+ * Copy a config
+ *
+ * @param fromConfig the config to copy from
+ * @param toConfig the config to copy to
+ * @throws IOException if an I/O error occurs
+ */
+ public abstract void copyConfigDir(String fromConfig, String toConfig)
throws IOException;
/**
- * The Solr standalone version of ConfigSetService.
+ * Copy a config
*
- * Loads a ConfigSet defined by the core's configSet property,
- * looking for a directory named for the configSet property value underneath
- * a base directory. If no configSet property is set, loads the ConfigSet
- * instead from the core's instance directory.
+ * @param fromConfig the config to copy from
+ * @param toConfig the config to copy to
+ * @param copiedToZkPaths should be an empty Set, will be filled in by
function
+ * with the paths that were actually copied to.
+ * @throws IOException if an I/O error occurs
*/
- public static class Standalone extends ConfigSetService {
+ public abstract void copyConfigDir(String fromConfig, String toConfig,
Set<String> copiedToZkPaths) throws IOException;
- private final Path configSetBase;
+ /**
+ * Upload files from a given path to a config
+ *
+ * @param dir {@link java.nio.file.Path} to the files
+ * @param configName the name to give the config
+ * @throws IOException if an I/O error occurs or the path does not exist
+ */
+ public abstract void uploadConfigDir(Path dir, String configName) throws
IOException;
- public Standalone(SolrResourceLoader loader, boolean shareSchema, Path
configSetBase) {
- super(loader, shareSchema);
- this.configSetBase = configSetBase;
- }
+ /**
+ * Download a config from Zookeeper and write it to the filesystem
+ *
+ * @param configName the config to download
+ * @param dir the {@link Path} to write files under
+ * @throws IOException if an I/O error occurs or the config does not exist
+ */
+ public abstract void downloadConfigDir(String configName, Path dir) throws
IOException;
- @Override
- public SolrResourceLoader createCoreResourceLoader(CoreDescriptor cd) {
- Path instanceDir = locateInstanceDir(cd);
- SolrResourceLoader solrResourceLoader = new
SolrResourceLoader(instanceDir, parentLoader.getClassLoader());
- return solrResourceLoader;
- }
+ public abstract List<String> listConfigs() throws IOException;
- @Override
- public String configSetName(CoreDescriptor cd) {
- return (cd.getConfigSet() == null ? "instancedir " : "configset ") +
locateInstanceDir(cd);
- }
+ public interface ConfigResource {
- protected Path locateInstanceDir(CoreDescriptor cd) {
- String configSet = cd.getConfigSet();
- if (configSet == null)
- return cd.getInstanceDir();
- Path configSetDirectory = configSetBase.resolve(configSet);
- if (!Files.isDirectory(configSetDirectory))
- throw new SolrException(SolrException.ErrorCode.SERVER_ERROR,
- "Could not load configuration from directory " +
configSetDirectory);
- return configSetDirectory;
- }
+ ConfigNode get() throws Exception;
- @Override
- protected Long getCurrentSchemaModificationVersion(String configSet,
SolrConfig solrConfig, String schemaFileName) {
- Path schemaFile =
Paths.get(solrConfig.getResourceLoader().getConfigDir()).resolve(schemaFileName);
- try {
- return Files.getLastModifiedTime(schemaFile).toMillis();
- } catch (FileNotFoundException e) {
- return null; // acceptable
- } catch (IOException e) {
- log.warn("Unexpected exception when getting modification time of {}",
schemaFile, e);
- return null; // debatable; we'll see an error soon if there's a real
problem
- }
- }
+ }
+ public static ConfigNode getParsedSchema(InputStream is, SolrResourceLoader
loader, String name) throws IOException, SAXException,
ParserConfigurationException {
Review comment:
This method seems out of place here... maybe move with the other static
methods in this class.
##########
File path:
solr/core/src/java/org/apache/solr/cloud/api/collections/CreateCollectionCmd.java
##########
@@ -626,7 +625,7 @@ private static void getConfName(DistribStateManager
stateManager, String collect
}
try {
- configNames = stateManager.listData(ZkConfigManager.CONFIGS_ZKNODE);
+ configNames = stateManager.listData(ZkConfigSetService.CONFIGS_ZKNODE);
Review comment:
No; use ConfigSetService.
##########
File path:
solr/core/src/java/org/apache/solr/core/FileSystemConfigSetService.java
##########
@@ -0,0 +1,119 @@
+/*
+ * 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.core;
+
+import java.io.FileNotFoundException;
+import java.io.IOException;
+import java.lang.invoke.MethodHandles;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.util.List;
+import java.util.Set;
+import java.util.stream.Collectors;
+
+import org.apache.solr.common.SolrException;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * The Solr standalone version of File System ConfigSetService.
+ * <p>
+ * Loads a ConfigSet defined by the core's configSet property,
+ * looking for a directory named for the configSet property value underneath
+ * a base directory. If no configSet property is set, loads the ConfigSet
+ * instead from the core's instance directory.
+ */
+public class FileSystemConfigSetService extends ConfigSetService {
+ private static final Logger log =
LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
+ private final Path configSetBase;
+
+ public FileSystemConfigSetService(CoreContainer cc) {
+ super(cc.getResourceLoader(), cc.getConfig().hasSchemaCache());
+ this.configSetBase = cc.getConfig().getConfigSetBaseDirectory();
+ }
+
+ @Override
+ public SolrResourceLoader createCoreResourceLoader(CoreDescriptor cd) {
+ Path instanceDir = locateInstanceDir(cd);
+ SolrResourceLoader solrResourceLoader = new
SolrResourceLoader(instanceDir, parentLoader.getClassLoader());
+ return solrResourceLoader;
+ }
+
+ @Override
+ public String configSetName(CoreDescriptor cd) {
+ return (cd.getConfigSet() == null ? "instancedir " : "configset ") +
locateInstanceDir(cd);
+ }
+
+ @Override
+ public void deleteConfigDir(String configName) throws IOException {
+ throw new UnsupportedOperationException("Only supported with
ZkConfigSetService");
+ }
+
+ @Override
+ public void copyConfigDir(String fromConfig, String toConfig) throws
IOException {
+ throw new UnsupportedOperationException("Only supported with
ZkConfigSetService");
+ }
+
+ @Override
+ public void copyConfigDir(String fromConfig, String toConfig, Set<String>
copiedToZkPaths) throws IOException {
+ throw new UnsupportedOperationException("Only supported with
ZkConfigSetService");
+ }
+
+ @Override
+ public void uploadConfigDir(Path dir, String configName) throws
IOException {
+ throw new UnsupportedOperationException("Only supported with
ZkConfigSetService");
+ }
+
+ @Override
+ public void downloadConfigDir(String configName, Path dir) throws
IOException {
+ throw new UnsupportedOperationException("Only supported with
ZkConfigSetService");
+ }
+
+ @Override
+ public List<String> listConfigs() throws IOException {
+ List<String> configs = Files.list(configSetBase)
Review comment:
return directly without declaring
##########
File path:
solr/core/src/test/org/apache/solr/cloud/api/collections/SimpleCollectionCreateDeleteTest.java
##########
@@ -141,7 +141,7 @@ public void
testDeleteDoesNotDeleteSharedAutocreatedConfigSet() throws Exception
// config for this collection is '.AUTOCREATED', and exists
globally
assertTrue(configName.endsWith(".AUTOCREATED"));
-
assertTrue(cloudClient.getZkStateReader().getConfigManager().listConfigs().contains(configName));
Review comment:
There are a number of calls like this in tests. The changed code is now
kind of worse but I understand why it couldn't stay the same. Perhaps
SolrZkClient should have a listConfigs() ?
##########
File path: solr/core/src/java/org/apache/solr/cloud/ZkConfigSetService.java
##########
@@ -0,0 +1,319 @@
+/*
+ * 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.cloud;
+
+import java.io.File;
+import java.io.IOException;
+import java.lang.invoke.MethodHandles;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.util.Collections;
+import java.util.List;
+import java.util.Locale;
+import java.util.Set;
+import java.util.regex.Pattern;
+
+import org.apache.solr.client.solrj.cloud.SolrCloudManager;
+import org.apache.solr.cloud.api.collections.CreateCollectionCmd;
+import org.apache.solr.common.SolrException;
+import org.apache.solr.common.cloud.SolrZkClient;
+import org.apache.solr.common.cloud.ZkStateReader;
+import org.apache.solr.common.cloud.ZooKeeperException;
+import org.apache.solr.common.util.NamedList;
+import org.apache.solr.core.ConfigSetProperties;
+import org.apache.solr.core.ConfigSetService;
+import org.apache.solr.core.CoreContainer;
+import org.apache.solr.core.CoreDescriptor;
+import org.apache.solr.core.SolrConfig;
+import org.apache.solr.core.SolrResourceLoader;
+import org.apache.solr.handler.admin.ConfigSetsHandler;
+import org.apache.solr.servlet.SolrDispatchFilter;
+import org.apache.zookeeper.KeeperException;
+import org.apache.zookeeper.data.Stat;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * SolrCloud Zookeeper ConfigSetService impl.
+ */
+public class ZkConfigSetService extends ConfigSetService {
+ private static final Logger log =
LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
+ private final ZkController zkController;
+ private final SolrZkClient zkClient;
+ /** ZkNode where named configs are stored */
+ public static final String CONFIGS_ZKNODE = "/configs";
+ public static final String UPLOAD_FILENAME_EXCLUDE_REGEX = "^\\..*$";
+ public static final Pattern UPLOAD_FILENAME_EXCLUDE_PATTERN =
Pattern.compile(UPLOAD_FILENAME_EXCLUDE_REGEX);
+
+ public ZkConfigSetService(CoreContainer cc) {
+ super(cc.getResourceLoader(), cc.getConfig().hasSchemaCache());
+ this.zkController = cc.getZkController();
+ this.zkClient = cc.getZkController().getZkClient();
+ try {
+ bootstrapDefaultConfigSet();
+ } catch (IOException e) {
+ log.error("Error in bootstrapping default config");
+ }
+ }
+
+ /** This is for ZkCLI and some tests */
+ public ZkConfigSetService(SolrZkClient zkClient) {
+ super(null, false);
+ this.zkController = null;
+ this.zkClient = zkClient;
+ }
+
+ @Override
+ public SolrResourceLoader createCoreResourceLoader(CoreDescriptor cd) {
+ final String colName = cd.getCollectionName();
+
+ // For back compat with cores that can create collections without the
collections API
+ try {
+ if (!zkClient.exists(ZkStateReader.COLLECTIONS_ZKNODE + "/" + colName,
true)) {
+ // TODO remove this functionality or maybe move to a CLI mechanism
+ log.warn("Auto-creating collection (in ZK) from core descriptor (on
disk). This feature may go away!");
+
CreateCollectionCmd.createCollectionZkNode(zkController.getSolrCloudManager().getDistribStateManager(),
colName, cd.getCloudDescriptor().getParams());
+ }
+ } catch (InterruptedException e) {
+ Thread.currentThread().interrupt();
+ throw new ZooKeeperException(SolrException.ErrorCode.SERVER_ERROR,
"Interrupted auto-creating collection", e);
+ } catch (KeeperException e) {
+ throw new ZooKeeperException(SolrException.ErrorCode.SERVER_ERROR,
"Failure auto-creating collection", e);
+ }
+
+ // 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);
+ 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);
+ }
+
+ @Override
+ @SuppressWarnings({"rawtypes"})
+ protected NamedList loadConfigSetFlags(CoreDescriptor cd, SolrResourceLoader
loader) {
+ try {
+ return ConfigSetProperties.readFromResourceLoader(loader, ".");
+ } catch (Exception ex) {
+ log.debug("No configSet flags", ex);
+ return null;
+ }
+ }
+
+ @Override
+ protected Long getCurrentSchemaModificationVersion(String configSet,
SolrConfig solrConfig, String schemaFile) {
+ String zkPath = CONFIGS_ZKNODE + "/" + configSet + "/" + schemaFile;
+ Stat stat;
+ try {
+ stat = zkClient.exists(zkPath, null, true);
+ } catch (KeeperException e) {
+ log.warn("Unexpected exception when getting modification time of {}",
zkPath, e);
+ return null; // debatable; we'll see an error soon if there's a real
problem
+ } catch (InterruptedException e) {
+ Thread.currentThread().interrupt();
+ throw new SolrException(SolrException.ErrorCode.SERVER_ERROR, e);
+ }
+ if (stat == null) { // not found
+ return null;
+ }
+ return (long) stat.getVersion();
+ }
+
+ @Override
+ public String configSetName(CoreDescriptor cd) {
+ return "configset " + cd.getConfigSet();
+ }
+
+ @Override
+ public boolean checkConfigExists(String configName) throws IOException {
+ try {
+ return zkClient.exists(CONFIGS_ZKNODE + "/" + configName, true);
+ } catch (KeeperException | InterruptedException e) {
+ throw new IOException("Error checking whether config exists",
+ SolrZkClient.checkInterrupted(e));
+ }
+ }
+
+ @Override
+ public void deleteConfigDir(String configName) throws IOException {
+ try {
+ zkClient.clean(CONFIGS_ZKNODE + "/" + configName);
+ } catch (KeeperException | InterruptedException e) {
+ throw new IOException("Error checking whether config exists",
+ SolrZkClient.checkInterrupted(e));
+ }
+ }
+
+ @Override
+ public void copyConfigDir(String fromConfig, String toConfig) throws
IOException {
+ copyConfigDir(fromConfig, toConfig, null);
+ }
+
+ @Override
+ public void copyConfigDir(String fromConfig, String toConfig, Set<String>
copiedToZkPaths) throws IOException {
+ String fromConfigPath = CONFIGS_ZKNODE + "/" + fromConfig;
+ String toConfigPath = CONFIGS_ZKNODE + "/" + toConfig;
+ try {
+ copyData(copiedToZkPaths, fromConfigPath, toConfigPath);
+ } catch (KeeperException | InterruptedException e) {
+ throw new IOException("Error config " + fromConfig + " to " + toConfig,
+ SolrZkClient.checkInterrupted(e));
+ }
+ copyConfigDirFromZk(fromConfigPath, toConfigPath, copiedToZkPaths);
+ }
+
+ @Override
+ public void uploadConfigDir(Path dir, String configName) throws IOException {
+ zkClient.uploadToZK(dir, CONFIGS_ZKNODE + "/" + configName,
ZkConfigSetService.UPLOAD_FILENAME_EXCLUDE_PATTERN);
+ }
+
+ @Override
+ public void downloadConfigDir(String configName, Path dir) throws
IOException {
+ zkClient.downloadFromZK(CONFIGS_ZKNODE + "/" + configName, dir);
+ }
+
+ @Override
+ public List<String> listConfigs() throws IOException {
+ try {
+ return zkClient.getChildren(CONFIGS_ZKNODE, null, true);
+ } catch (KeeperException.NoNodeException e) {
+ return Collections.emptyList();
+ } catch (KeeperException | InterruptedException e) {
+ throw new IOException("Error listing configs",
SolrZkClient.checkInterrupted(e));
+ }
+ }
+
+ private void bootstrapDefaultConfigSet() throws IOException {
+ if (this.checkConfigExists("_default") == false) {
+ String configDirPath = getDefaultConfigDirPath();
+ if (configDirPath == null) {
+ log.warn(
+ "The _default configset could not be uploaded. Please provide
'solr.default.confdir' parameter that points to a configset {} {}",
+ "intended to be the default. Current 'solr.default.confdir'
value:",
+
System.getProperty(SolrDispatchFilter.SOLR_DEFAULT_CONFDIR_ATTRIBUTE));
+ } else {
+ this.uploadConfigDir(Paths.get(configDirPath),
ConfigSetsHandler.DEFAULT_CONFIGSET_NAME);
+ }
+ }
+ }
+
+ /**
+ * Gets the absolute filesystem path of the _default configset to bootstrap
from. First tries the
+ * sysprop "solr.default.confdir". If not found, tries to find the _default
dir relative to the
+ * sysprop "solr.install.dir". Returns null if not found anywhere.
+ *
+ * @lucene.internal
+ * @see SolrDispatchFilter#SOLR_DEFAULT_CONFDIR_ATTRIBUTE
+ */
+ public static String getDefaultConfigDirPath() {
Review comment:
Looks like it belongs on the FileSystem one? This isn't ZK related.
##########
File path:
solr/core/src/java/org/apache/solr/cloud/api/collections/OverseerCollectionMessageHandler.java
##########
@@ -67,6 +68,7 @@
HttpShardHandlerFactory shardHandlerFactory;
String adminPath;
ZkStateReader zkStateReader;
+ CoreContainer cc;
Review comment:
Not used?
##########
File path:
solr/core/src/java/org/apache/solr/cloud/OverseerCollectionConfigSetProcessor.java
##########
@@ -88,9 +88,9 @@ private static OverseerMessageHandlerSelector
getOverseerMessageHandlerSelector(
Overseer overseer,
OverseerNodePrioritizer overseerNodePrioritizer) {
final OverseerCollectionMessageHandler collMessageHandler = new
OverseerCollectionMessageHandler(
- zkStateReader, myId, shardHandlerFactory, adminPath, stats, overseer,
overseerNodePrioritizer);
+ zkStateReader, myId, shardHandlerFactory, adminPath, stats, overseer,
overseerNodePrioritizer, overseer.getCoreContainer());
Review comment:
Why does it need this?
##########
File path:
solr/core/src/java/org/apache/solr/cloud/api/collections/CreateCollectionCmd.java
##########
@@ -476,7 +477,7 @@ String getConfigName(String coll, ZkNodeProps message)
throws KeeperException, I
// if there is only one conf, use that
List<String> configNames = null;
try {
- configNames =
ccc.getZkStateReader().getZkClient().getChildren(ZkConfigManager.CONFIGS_ZKNODE,
null, true);
Review comment:
No; should be using configSetService to list configs
##########
File path:
solr/core/src/java/org/apache/solr/cloud/OverseerConfigSetMessageHandler.java
##########
@@ -70,6 +70,7 @@
public static final String PROPERTY_PREFIX = "configSetProp";
private ZkStateReader zkStateReader;
+ private CoreContainer cc;
Review comment:
ConfigSetService instead
##########
File path:
solr/core/src/test/org/apache/solr/cloud/TestConfigSetsAPIZkFailure.java
##########
@@ -111,14 +110,14 @@ public void testCreateZkFailure() throws Exception {
SolrZkClient zkClient = new
SolrZkClient(solrCluster.getZkServer().getZkAddress(),
AbstractZkTestCase.TIMEOUT, AbstractZkTestCase.TIMEOUT, null);
try {
- ZkConfigManager configManager = new ZkConfigManager(zkClient);
- assertFalse(configManager.configExists(CONFIGSET_NAME));
+
+
assertFalse(solrCluster.getOpenOverseer().getCoreContainer().getConfigSetService().checkConfigExists(CONFIGSET_NAME));
Review comment:
long and used twice. Declare
`solrCluster.getOpenOverseer().getCoreContainer().getConfigSetService()`
##########
File path:
solr/core/src/test/org/apache/solr/cloud/api/collections/TestCollectionAPI.java
##########
@@ -230,7 +230,7 @@ private void testNoConfigset() throws Exception {
waitForRecoveriesToFinish(collection, false);
// Now try deleting the configset and doing a clusterstatus.
- String parent = ZkConfigManager.CONFIGS_ZKNODE + "/" + configSet;
+ String parent = ZkConfigSetService.CONFIGS_ZKNODE + "/" + configSet;
Review comment:
Could we create a ZkConfigSetService to call convenient APIs for this?
##########
File path: solr/solr-ref-guide/src/format-of-solr-xml.adoc
##########
@@ -68,7 +68,7 @@ This attribute does not need to be set.
+
If used, this attribute should be set to the FQN (Fully qualified name) of a
class that inherits from ConfigSetService , and you must provide a constructor
with one param which the type is `org.apache.solr.core.CoreContainer` . For
example, `<str name="configSetService">com.myorg.CustomConfigSetService</str>`.
+
-If this attribute isn't set, Solr uses the default configSetService , with
zookeeper aware of `org.apache.solr.cloud.CloudConfigSetService`, without
zookeeper aware of `org.apache.solr.core.ConfigSetService.Standalone`.
+If this attribute isn't set, Solr uses the default configSetService , with
zookeeper aware of `org.apache.solr.cloud.ZkConfigSetService`, without
zookeeper aware of
`org.apache.solr.core.ConfigSetService.FileSystemConfigSetService`.
Review comment:
The FS one is no longer an inner class.
##########
File path: solr/core/src/test/org/apache/solr/cloud/TestConfigSetsAPI.java
##########
@@ -293,8 +292,7 @@ private NamedList getConfigSetPropertiesFromZk(
private void verifyProperties(String configSetName, Map<String, String>
oldProps,
Map<String, String> newProps, SolrZkClient zkClient) throws Exception {
@SuppressWarnings({"rawtypes"})
- NamedList properties = getConfigSetPropertiesFromZk(zkClient,
- ZkConfigManager.CONFIGS_ZKNODE + "/" + configSetName + "/" +
DEFAULT_FILENAME);
+ NamedList properties =
getConfigSetPropertiesFromZk(zkClient,ZkConfigSetService.CONFIGS_ZKNODE + "/" +
configSetName + "/" + DEFAULT_FILENAME);
Review comment:
`getConfigSetPropertiesFromZk` circumvents the ConfigSetService; could
it use it?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
