diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt index bbac86c4c16..6474707d929 100644 --- a/solr/CHANGES.txt +++ b/solr/CHANGES.txt @@ -132,6 +132,8 @@ Optimizations * SOLR-17099: Do not return spurious tags when fetching metrics from a Solr node to another. (Pierre Salagnac) +* SOLR-17269: Prevent the "Coordinator node" feature from registering synthetic cores in ZooKeeper (Patson Luk) + Bug Fixes --------------------- * SOLR-12813: subqueries should respect basic auth. (Rudy Seitz via Eric Pugh) 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 138d729d9ee..5057e62613f 100644 --- a/solr/core/src/java/org/apache/solr/cloud/ZkConfigSetService.java +++ b/solr/core/src/java/org/apache/solr/cloud/ZkConfigSetService.java @@ -26,12 +26,9 @@ import java.util.Map; import java.util.Objects; 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.ZkMaintenanceUtils; -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.common.util.Utils; import org.apache.solr.core.ConfigSetProperties; @@ -71,36 +68,18 @@ public ZkConfigSetService(SolrZkClient 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(), - zkController.getCoreContainer().getConfigSetService()); - } - } 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); + // Currently, cd.getConfigSet() is always null. Except that it's explicitly set by + // {@link org.apache.solr.core.SyntheticSolrCore}. + // Should we consider setting it for all cores as a part of CoreDescriptor creation/loading + // process? + if (cd.getConfigSet() == null) { + String configSetName = + zkController.getClusterState().getCollection(cd.getCollectionName()).getConfigName(); + cd.setConfigSet(configSetName); } - // The configSet is read from ZK and populated. Ignore CD's pre-existing configSet; only - // populated in standalone - String configSetName = zkController.getClusterState().getCollection(colName).getConfigName(); - cd.setConfigSet(configSetName); - return new ZkSolrResourceLoader( - cd.getInstanceDir(), configSetName, parentLoader.getClassLoader(), zkController); + cd.getInstanceDir(), cd.getConfigSet(), parentLoader.getClassLoader(), zkController); } @Override diff --git a/solr/core/src/java/org/apache/solr/core/SolrCore.java b/solr/core/src/java/org/apache/solr/core/SolrCore.java index 92a963f6bbe..1a53a3db8a3 100644 --- a/solr/core/src/java/org/apache/solr/core/SolrCore.java +++ b/solr/core/src/java/org/apache/solr/core/SolrCore.java @@ -1235,7 +1235,7 @@ private SolrCore( } /** Set UpdateLog to buffer updates if the slice is in construction. */ - private void bufferUpdatesIfConstructing(CoreDescriptor coreDescriptor) { + protected void bufferUpdatesIfConstructing(CoreDescriptor coreDescriptor) { if (coreContainer != null && coreContainer.isZooKeeperAware()) { if (reqHandlers.get("/get") == null) { diff --git a/solr/core/src/java/org/apache/solr/core/SyntheticSolrCore.java b/solr/core/src/java/org/apache/solr/core/SyntheticSolrCore.java new file mode 100644 index 00000000000..e97c5780d54 --- /dev/null +++ b/solr/core/src/java/org/apache/solr/core/SyntheticSolrCore.java @@ -0,0 +1,75 @@ +/* + * 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.nio.file.Path; +import java.util.Map; +import org.apache.solr.common.SolrException; +import org.apache.solr.common.params.CoreAdminParams; +import org.apache.solr.rest.RestManager; + +/** + * A synthetic core that is created only in memory and not registered against Zookeeper. + * + *

This is only used in Coordinator node to support a subset of SolrCore functionalities required + * by Coordinator operations such as aggregating and writing out response and providing configset + * info. + * + *

There should only be one instance of SyntheticSolrCore per configset + */ +public class SyntheticSolrCore extends SolrCore { + public SyntheticSolrCore(CoreContainer coreContainer, CoreDescriptor cd, ConfigSet configSet) { + super(coreContainer, cd, configSet); + } + + public static SyntheticSolrCore createAndRegisterCore( + CoreContainer coreContainer, String syntheticCoreName, String configSetName) { + Map coreProps = Map.of(CoreAdminParams.COLLECTION, syntheticCoreName); + + CoreDescriptor syntheticCoreDescriptor = + new CoreDescriptor( + syntheticCoreName, + Path.of(coreContainer.getSolrHome(), syntheticCoreName), + coreProps, + coreContainer.getContainerProperties(), + coreContainer.getZkController()); + syntheticCoreDescriptor.setConfigSet(configSetName); + ConfigSet coreConfig = + coreContainer.getConfigSetService().loadConfigSet(syntheticCoreDescriptor); + syntheticCoreDescriptor.setConfigSetTrusted(coreConfig.isTrusted()); + SyntheticSolrCore syntheticCore = + new SyntheticSolrCore(coreContainer, syntheticCoreDescriptor, coreConfig); + coreContainer.registerCore(syntheticCoreDescriptor, syntheticCore, false, false); + + return syntheticCore; + } + + @Override + protected void bufferUpdatesIfConstructing(CoreDescriptor coreDescriptor) { + // no updates to SyntheticSolrCore + } + + @Override + protected RestManager initRestManager() throws SolrException { + // returns an initialized RestManager. As init routines requires reading configname of the + // core's collection from ZK + // which synthetic core is not registered in ZK. + // We do not expect RestManager ops on Coordinator Nodes + return new RestManager(); + } +} diff --git a/solr/core/src/java/org/apache/solr/servlet/CoordinatorHttpSolrCall.java b/solr/core/src/java/org/apache/solr/servlet/CoordinatorHttpSolrCall.java index ec00107b717..da8ca523fce 100644 --- a/solr/core/src/java/org/apache/solr/servlet/CoordinatorHttpSolrCall.java +++ b/solr/core/src/java/org/apache/solr/servlet/CoordinatorHttpSolrCall.java @@ -18,32 +18,25 @@ package org.apache.solr.servlet; import java.lang.invoke.MethodHandles; -import java.util.List; import java.util.Map; import java.util.Properties; import java.util.concurrent.ConcurrentHashMap; -import java.util.concurrent.TimeUnit; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; import org.apache.solr.api.CoordinatorV2HttpSolrCall; -import org.apache.solr.client.solrj.request.CollectionAdminRequest; import org.apache.solr.cloud.CloudDescriptor; import org.apache.solr.cloud.api.collections.Assign; -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.Replica; import org.apache.solr.common.cloud.ZkStateReader; -import org.apache.solr.common.params.SolrParams; -import org.apache.solr.common.util.Utils; import org.apache.solr.core.CoreContainer; import org.apache.solr.core.CoreDescriptor; import org.apache.solr.core.SolrCore; +import org.apache.solr.core.SyntheticSolrCore; import org.apache.solr.logging.MDCLoggingContext; import org.apache.solr.request.DelegatingSolrQueryRequest; -import org.apache.solr.request.LocalSolrQueryRequest; import org.apache.solr.request.SolrQueryRequest; -import org.apache.solr.response.SolrQueryResponse; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -88,198 +81,70 @@ public static SolrCore getCore( String syntheticCoreName = factory.collectionVsCoreNameMapping.get(collectionName); if (syntheticCoreName != null) { SolrCore syntheticCore = solrCall.cores.getCore(syntheticCoreName); - setMDCLoggingContext(collectionName); + setMdcLoggingContext(collectionName); return syntheticCore; } else { + // first time loading this collection ZkStateReader zkStateReader = solrCall.cores.getZkController().getZkStateReader(); ClusterState clusterState = zkStateReader.getClusterState(); DocCollection coll = clusterState.getCollectionOrNull(collectionName, true); - SolrCore core = null; - if (coll != null) { - String confName = coll.getConfigName(); - String syntheticCollectionName = getSyntheticCollectionName(confName); - DocCollection syntheticColl = clusterState.getCollectionOrNull(syntheticCollectionName); - synchronized (CoordinatorHttpSolrCall.class) { - if (syntheticColl == null) { - // no synthetic collection for this config, let's create one - if (log.isInfoEnabled()) { - log.info( - "synthetic collection: {} does not exist, creating.. ", syntheticCollectionName); - } - - SolrException createException = null; - try { - createColl(syntheticCollectionName, solrCall.cores, confName); - } catch (SolrException exception) { - // concurrent requests could have created the collection hence causing collection - // exists - // exception - createException = exception; - } finally { - syntheticColl = - zkStateReader.getClusterState().getCollectionOrNull(syntheticCollectionName); - } - - // then indeed the collection was not created properly, either by this or other - // concurrent - // requests - if (syntheticColl == null) { - if (createException != null) { - throw createException; // rethrow the exception since such collection was not - // created - } else { - throw new SolrException( - SolrException.ErrorCode.SERVER_ERROR, - "Could not locate synthetic collection [" - + syntheticCollectionName - + "] after creation!"); - } - } - } - - // get docCollection again to ensure we get the fresh state - syntheticColl = - zkStateReader.getClusterState().getCollectionOrNull(syntheticCollectionName); - List nodeNameSyntheticReplicas = - syntheticColl.getReplicas(solrCall.cores.getZkController().getNodeName()); - if (nodeNameSyntheticReplicas == null || nodeNameSyntheticReplicas.isEmpty()) { - // this node does not have a replica. add one - if (log.isInfoEnabled()) { - log.info( - "this node does not have a replica of the synthetic collection: {} , adding replica ", - syntheticCollectionName); - } + if (coll == null) { // querying on a non-existent collection, it could have been removed + log.info( + "Cannot find collection {} to proxy call to, it could have been deleted", + collectionName); + return null; + } - addReplica(syntheticCollectionName, solrCall.cores); - } + String confName = coll.getConfigName(); + syntheticCoreName = getSyntheticCoreNameFromConfig(confName); + + SolrCore syntheticCore; + synchronized (CoordinatorHttpSolrCall.class) { + CoreContainer coreContainer = solrCall.cores; + syntheticCore = coreContainer.getCore(syntheticCoreName); + if (syntheticCore == null) { + // first time loading this config set + log.info("Loading synthetic core for config set {}", confName); + syntheticCore = + SyntheticSolrCore.createAndRegisterCore( + coreContainer, syntheticCoreName, coll.getConfigName()); + // getting the core should open it + syntheticCore.open(); + } - // still have to ensure that it's active, otherwise super.getCoreByCollection - // will return null and then CoordinatorHttpSolrCall will call getCore again - // hence creating a calling loop - try { - zkStateReader.waitForState( - syntheticCollectionName, - 10, - TimeUnit.SECONDS, - docCollection -> { - List replicas = - docCollection.getReplicas(solrCall.cores.getZkController().getNodeName()); - if (replicas == null || replicas.isEmpty()) { + factory.collectionVsCoreNameMapping.put(collectionName, syntheticCore.getName()); + + // for the watcher, only remove on collection deletion (ie collection == null), since + // watch from coordinator is collection specific + coreContainer + .getZkController() + .getZkStateReader() + .registerDocCollectionWatcher( + collectionName, + collection -> { + if (collection == null) { + factory.collectionVsCoreNameMapping.remove(collectionName); + return true; + } else { return false; } - for (Replica nodeNameSyntheticReplica : replicas) { - if (nodeNameSyntheticReplica.getState() == Replica.State.ACTIVE) { - return true; - } - } - return false; }); - } catch (Exception e) { - throw new SolrException( - SolrException.ErrorCode.SERVER_ERROR, - "Failed to wait for active replica for synthetic collection [" - + syntheticCollectionName - + "]", - e); - } - } - - core = solrCall.getCoreByCollection(syntheticCollectionName, isPreferLeader); - if (core != null) { - factory.collectionVsCoreNameMapping.put(collectionName, core.getName()); - // for the watcher, only remove on collection deletion (ie collection == null), since - // watch from coordinator is collection specific - solrCall - .cores - .getZkController() - .getZkStateReader() - .registerDocCollectionWatcher( - collectionName, - collection -> { - if (collection == null) { - factory.collectionVsCoreNameMapping.remove(collectionName); - return true; - } else { - return false; - } - }); - if (log.isDebugEnabled()) { - log.debug("coordinator node, returns synthetic core: {}", core.getName()); - } - } - setMDCLoggingContext(collectionName); - return core; } - return null; + setMdcLoggingContext(collectionName); + if (log.isDebugEnabled()) { + log.debug("coordinator node, returns synthetic core: {}", syntheticCore.getName()); + } + return syntheticCore; } } - public static String getSyntheticCollectionName(String configName) { + public static String getSyntheticCollectionNameFromConfig(String configName) { return SYNTHETIC_COLL_PREFIX + configName; } - /** - * Overrides the MDC context as the core set was synthetic core, which does not reflect the - * collection being operated on - */ - private static void setMDCLoggingContext(String collectionName) { - MDCLoggingContext.setCollection(collectionName); - - // below is irrelevant for call to coordinator - MDCLoggingContext.setCoreName(null); - MDCLoggingContext.setShard(null); - MDCLoggingContext.setCoreName(null); - } - - private static void addReplica(String syntheticCollectionName, CoreContainer cores) { - SolrQueryResponse rsp = new SolrQueryResponse(); - try { - CollectionAdminRequest.AddReplica addReplicaRequest = - CollectionAdminRequest.addReplicaToShard(syntheticCollectionName, "shard1") - // we are fixing the name, so that no two replicas are created in the same node - .setNode(cores.getZkController().getNodeName()); - addReplicaRequest.setWaitForFinalState(true); - cores - .getCollectionsHandler() - .handleRequestBody(new LocalSolrQueryRequest(null, addReplicaRequest.getParams()), rsp); - if (rsp.getValues().get("success") == null) { - throw new SolrException( - SolrException.ErrorCode.SERVER_ERROR, - "Could not auto-create collection: " + Utils.toJSONString(rsp.getValues())); - } - } catch (Exception e) { - throw new SolrException(SolrException.ErrorCode.SERVER_ERROR, e); - } - } - - private static void createColl( - String syntheticCollectionName, CoreContainer cores, String confName) { - SolrQueryResponse rsp = new SolrQueryResponse(); - try { - CollectionAdminRequest.Create collCreationRequest = - CollectionAdminRequest.createCollection(syntheticCollectionName, confName, 1, 1) - .setCreateNodeSet(cores.getZkController().getNodeName()); - collCreationRequest.setWaitForFinalState(true); - SolrParams params = collCreationRequest.getParams(); - if (log.isInfoEnabled()) { - log.info("sending collection admin command : {}", Utils.toJSONString(params)); - } - cores.getCollectionsHandler().handleRequestBody(new LocalSolrQueryRequest(null, params), rsp); - if (rsp.getValues().get("success") == null) { - throw new SolrException( - SolrException.ErrorCode.SERVER_ERROR, - "Could not create :" - + syntheticCollectionName - + " collection: " - + Utils.toJSONString(rsp.getValues())); - } - } catch (SolrException e) { - throw e; - - } catch (Exception e) { - throw new SolrException(SolrException.ErrorCode.SERVER_ERROR, e); - } + public static String getSyntheticCoreNameFromConfig(String configName) { + return getSyntheticCollectionNameFromConfig(configName) + "_core"; } @Override @@ -298,7 +163,9 @@ protected String getCoreOrColName() { public static SolrQueryRequest wrappedReq( SolrQueryRequest delegate, String collectionName, HttpSolrCall httpSolrCall) { Properties p = new Properties(); - p.put(CoreDescriptor.CORE_COLLECTION, collectionName); + if (collectionName != null) { + p.put(CoreDescriptor.CORE_COLLECTION, collectionName); + } p.put(CloudDescriptor.REPLICA_TYPE, Replica.Type.PULL.toString()); p.put(CoreDescriptor.CORE_SHARD, "_"); @@ -318,6 +185,19 @@ public CloudDescriptor getCloudDescriptor() { }; } + /** + * Overrides the MDC context as the core set was synthetic core, which does not reflect the + * collection being operated on + */ + private static void setMdcLoggingContext(String collectionName) { + MDCLoggingContext.setCollection(collectionName); + + // below is irrelevant for call to coordinator + MDCLoggingContext.setCoreName(null); + MDCLoggingContext.setShard(null); + MDCLoggingContext.setCoreName(null); + } + // The factory that creates an instance of HttpSolrCall public static class Factory implements SolrDispatchFilter.HttpSolrCallFactory { private final Map collectionVsCoreNameMapping = new ConcurrentHashMap<>(); diff --git a/solr/core/src/test/org/apache/solr/search/TestCoordinatorRole.java b/solr/core/src/test/org/apache/solr/search/TestCoordinatorRole.java index f55aca49ae8..70835d7e884 100644 --- a/solr/core/src/test/org/apache/solr/search/TestCoordinatorRole.java +++ b/solr/core/src/test/org/apache/solr/search/TestCoordinatorRole.java @@ -24,11 +24,9 @@ import java.util.ArrayList; import java.util.Date; import java.util.EnumSet; -import java.util.HashSet; import java.util.List; import java.util.Objects; import java.util.Random; -import java.util.Set; import java.util.concurrent.ExecutorService; import java.util.concurrent.Future; import java.util.concurrent.TimeUnit; @@ -62,6 +60,8 @@ import org.apache.solr.common.util.ExecutorUtil; import org.apache.solr.common.util.SolrNamedThreadFactory; import org.apache.solr.core.NodeRoles; +import org.apache.solr.core.SolrCore; +import org.apache.solr.core.SyntheticSolrCore; import org.apache.solr.embedded.JettySolrRunner; import org.apache.solr.servlet.CoordinatorHttpSolrCall; import org.slf4j.Logger; @@ -76,7 +76,6 @@ public void testSimple() throws Exception { try { CloudSolrClient client = cluster.getSolrClient(); String COLLECTION_NAME = "test_coll"; - String SYNTHETIC_COLLECTION = CoordinatorHttpSolrCall.getSyntheticCollectionName("conf"); CollectionAdminRequest.createCollection(COLLECTION_NAME, "conf", 2, 2) .process(cluster.getSolrClient()); cluster.waitForActiveCollection(COLLECTION_NAME, 2, 4); @@ -105,14 +104,20 @@ public void testSimple() throws Exception { assertEquals(10, rslt.getResults().size()); + String SYNTHETIC_COLLECTION = + CoordinatorHttpSolrCall.getSyntheticCollectionNameFromConfig("conf"); DocCollection collection = cluster.getSolrClient().getClusterStateProvider().getCollection(SYNTHETIC_COLLECTION); - assertNotNull(collection); - - Set expectedNodes = new HashSet<>(); - expectedNodes.add(coordinatorJetty.getNodeName()); - collection.forEachReplica((s, replica) -> expectedNodes.remove(replica.getNodeName())); - assertTrue(expectedNodes.isEmpty()); + // this should be empty as synthetic collection does not register with ZK + assertNull(collection); + + String syntheticCoreName = CoordinatorHttpSolrCall.getSyntheticCoreNameFromConfig("conf"); + try (SolrCore syntheticCore = + coordinatorJetty.getCoreContainer().getCore(syntheticCoreName)) { + assertNotNull(syntheticCore); + assertTrue(syntheticCore instanceof SyntheticSolrCore); + assertEquals("conf", syntheticCore.getCoreDescriptor().getConfigSet()); + } } finally { cluster.shutdown(); } @@ -124,7 +129,6 @@ public void testMultiCollectionMultiNode() throws Exception { try { CloudSolrClient client = cluster.getSolrClient(); String COLLECTION_NAME = "test_coll"; - String SYNTHETIC_COLLECTION = CoordinatorHttpSolrCall.getSyntheticCollectionName("conf"); for (int j = 1; j <= 10; j++) { String collname = COLLECTION_NAME + "_" + j; CollectionAdminRequest.createCollection(collname, "conf", 2, 2) @@ -170,15 +174,6 @@ public void testMultiCollectionMultiNode() throws Exception { assertEquals(10, rslt.getResults().size()); } - - DocCollection collection = - cluster.getSolrClient().getClusterStateProvider().getCollection(SYNTHETIC_COLLECTION); - assertNotNull(collection); - - int coordNode1NumCores = coordinatorJetty1.getCoreContainer().getNumAllCores(); - assertEquals("Unexpected number of cores found for coordinator node", 1, coordNode1NumCores); - int coordNode2NumCores = coordinatorJetty2.getCoreContainer().getNumAllCores(); - assertEquals("Unexpected number of cores found for coordinator node", 1, coordNode2NumCores); } finally { cluster.shutdown(); } @@ -573,15 +568,6 @@ public void testConcurrentAccess() throws Exception { testFuture.get(); // check for any exceptions/failures } - // number of replicas created in the synthetic collection should be one per coordinator node - assertEquals( - COORDINATOR_NODE_COUNT, - client - .getClusterState() - .getCollection(CoordinatorHttpSolrCall.getSyntheticCollectionName("conf")) - .getReplicas() - .size()); - executorService.shutdown(); executorService.awaitTermination(10, TimeUnit.SECONDS); } finally {