From 7e2ffb627379da7561bf32366ca52bab34c43ba8 Mon Sep 17 00:00:00 2001 From: patsonluk Date: Fri, 23 Feb 2024 18:36:10 -0800 Subject: [PATCH 01/20] Experimental QA core as proxy in memory --- .../org/apache/solr/core/CoreContainer.java | 2 +- .../solr/servlet/CoordinatorHttpSolrCall.java | 22 +++++++++++++++++++ 2 files changed, 23 insertions(+), 1 deletion(-) diff --git a/solr/core/src/java/org/apache/solr/core/CoreContainer.java b/solr/core/src/java/org/apache/solr/core/CoreContainer.java index 2aae7e733b9..0fc202b0c5b 100644 --- a/solr/core/src/java/org/apache/solr/core/CoreContainer.java +++ b/solr/core/src/java/org/apache/solr/core/CoreContainer.java @@ -1644,7 +1644,7 @@ public AllowListUrlChecker getAllowListUrlChecker() { * @return the newly created core */ @SuppressWarnings("resource") - private SolrCore createFromDescriptor( + public SolrCore createFromDescriptor( CoreDescriptor dcore, boolean publishState, boolean newCollection) { if (isShutDown) { 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 82664b23623..424dd2a4665 100644 --- a/solr/core/src/java/org/apache/solr/servlet/CoordinatorHttpSolrCall.java +++ b/solr/core/src/java/org/apache/solr/servlet/CoordinatorHttpSolrCall.java @@ -18,6 +18,8 @@ package org.apache.solr.servlet; import java.lang.invoke.MethodHandles; +import java.nio.file.Paths; +import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.Properties; @@ -34,6 +36,7 @@ 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.CoreAdminParams; import org.apache.solr.common.params.SolrParams; import org.apache.solr.common.util.Utils; import org.apache.solr.core.CoreContainer; @@ -90,6 +93,25 @@ public static SolrCore getCore( String confName = coll.getConfigName(); String syntheticCollectionName = getSyntheticCollectionName(confName); + CoreContainer coreContainer = solrCall.cores; + Map coreProps = new HashMap<>(); + coreProps.put(CoreAdminParams.CORE_NODE_NAME, coreContainer.getHostName()); + coreProps.put(CoreAdminParams.COLLECTION, syntheticCollectionName); + + CoreDescriptor syntheticCoreDescriptor = new CoreDescriptor( + collectionName, + Paths.get(coreContainer.getSolrHome() + "/" + collectionName), + coreProps, coreContainer.getContainerProperties(), coreContainer.getZkController()); + + SolrCore syntheticCore = coreContainer.createFromDescriptor(syntheticCoreDescriptor, false, false); + + //after this point the sync core should be available in the container. Double check + if (coreContainer.getCore(syntheticCore.getName()) != null) { + factory.collectionVsCoreNameMapping.put(collectionName, core.getName()); + return syntheticCore; + } + + DocCollection syntheticColl = clusterState.getCollectionOrNull(syntheticCollectionName); synchronized (CoordinatorHttpSolrCall.class) { if (syntheticColl == null) { From f6ce0c1c5ed3df823e563b85e733d28ce0a994a5 Mon Sep 17 00:00:00 2001 From: patsonluk Date: Fri, 23 Feb 2024 20:13:19 -0800 Subject: [PATCH 02/20] Experimental QA core as proxy in memory --- .../java/org/apache/solr/core/ConfigSetService.java | 11 +++++++++-- .../src/java/org/apache/solr/core/CoreContainer.java | 3 ++- .../apache/solr/servlet/CoordinatorHttpSolrCall.java | 8 +++++++- 3 files changed, 18 insertions(+), 4 deletions(-) diff --git a/solr/core/src/java/org/apache/solr/core/ConfigSetService.java b/solr/core/src/java/org/apache/solr/core/ConfigSetService.java index 32432d9f18b..b767d77c685 100644 --- a/solr/core/src/java/org/apache/solr/core/ConfigSetService.java +++ b/solr/core/src/java/org/apache/solr/core/ConfigSetService.java @@ -244,15 +244,19 @@ public boolean isConfigSetTrusted(SolrResourceLoader coreLoader) throws IOExcept return (flags == null || flags.get("trusted") == null || flags.getBooleanArg("trusted")); } + public final ConfigSet loadConfigSet(CoreDescriptor dcore) { + return loadConfigSet(dcore, null); + } + /** * Load the ConfigSet for a core * * @param dcore the core's CoreDescriptor * @return a ConfigSet */ - public final ConfigSet loadConfigSet(CoreDescriptor dcore) { + public final ConfigSet loadConfigSet(CoreDescriptor dcore, String configSetName) { - SolrResourceLoader coreLoader = createCoreResourceLoader(dcore); + SolrResourceLoader coreLoader = configSetName != null ? createConfigSetResourceLoader(dcore, configSetName) : createCoreResourceLoader(dcore); try { // ConfigSet properties are loaded from ConfigSetProperties.DEFAULT_FILENAME file. @@ -391,6 +395,9 @@ protected NamedList loadConfigSetFlags(SolrResourceLoader loader) throws */ protected abstract SolrResourceLoader createCoreResourceLoader(CoreDescriptor cd); + protected SolrResourceLoader createConfigSetResourceLoader(CoreDescriptor cd, String configSetName) { + throw new UnsupportedOperationException("Not supported"); + } /** * Return a name for the ConfigSet for a core to be used for printing/diagnostic purposes. * diff --git a/solr/core/src/java/org/apache/solr/core/CoreContainer.java b/solr/core/src/java/org/apache/solr/core/CoreContainer.java index 0fc202b0c5b..4c44f29a890 100644 --- a/solr/core/src/java/org/apache/solr/core/CoreContainer.java +++ b/solr/core/src/java/org/apache/solr/core/CoreContainer.java @@ -1433,7 +1433,8 @@ public CoresLocator getCoresLocator() { return coresLocator; } - protected SolrCore registerCore( + //TODO was protected, should go with new class approach + public SolrCore registerCore( CoreDescriptor cd, SolrCore core, boolean registerInZk, boolean skipRecovery) { if (core == null) { throw new RuntimeException("Can not register a null core."); 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 424dd2a4665..a2759b3edb8 100644 --- a/solr/core/src/java/org/apache/solr/servlet/CoordinatorHttpSolrCall.java +++ b/solr/core/src/java/org/apache/solr/servlet/CoordinatorHttpSolrCall.java @@ -39,6 +39,7 @@ import org.apache.solr.common.params.CoreAdminParams; import org.apache.solr.common.params.SolrParams; import org.apache.solr.common.util.Utils; +import org.apache.solr.core.ConfigSet; import org.apache.solr.core.CoreContainer; import org.apache.solr.core.CoreDescriptor; import org.apache.solr.core.SolrCore; @@ -103,7 +104,12 @@ public static SolrCore getCore( Paths.get(coreContainer.getSolrHome() + "/" + collectionName), coreProps, coreContainer.getContainerProperties(), coreContainer.getZkController()); - SolrCore syntheticCore = coreContainer.createFromDescriptor(syntheticCoreDescriptor, false, false); + + ConfigSet coreConfig = coreContainer.getConfigSetService().loadConfigSet(syntheticCoreDescriptor, confName); + syntheticCoreDescriptor.setConfigSetTrusted(coreConfig.isTrusted()); + SolrCore syntheticCore = new SolrCore(coreContainer, syntheticCoreDescriptor, coreConfig); + + coreContainer.registerCore(syntheticCoreDescriptor, syntheticCore, false, false); //after this point the sync core should be available in the container. Double check if (coreContainer.getCore(syntheticCore.getName()) != null) { From 64b529346e9cc16725e5aef70ec3f01211fc37d0 Mon Sep 17 00:00:00 2001 From: patsonluk Date: Fri, 23 Feb 2024 20:32:57 -0800 Subject: [PATCH 03/20] Experimental QA core as proxy in memory --- .../apache/solr/cloud/ZkConfigSetService.java | 6 + .../java/org/apache/solr/core/SolrCore.java | 15 ++- .../solr/servlet/CoordinatorHttpSolrCall.java | 119 +----------------- 3 files changed, 24 insertions(+), 116 deletions(-) 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 5f83f88c8eb..86a088ecef7 100644 --- a/solr/core/src/java/org/apache/solr/cloud/ZkConfigSetService.java +++ b/solr/core/src/java/org/apache/solr/cloud/ZkConfigSetService.java @@ -66,6 +66,12 @@ public ZkConfigSetService(SolrZkClient zkClient) { this.zkClient = zkClient; } + @Override + protected SolrResourceLoader createConfigSetResourceLoader(CoreDescriptor cd, String configSetName) { + return new ZkSolrResourceLoader( + cd.getInstanceDir(), configSetName, parentLoader.getClassLoader(), zkController); + } + @Override public SolrResourceLoader createCoreResourceLoader(CoreDescriptor cd) { final String colName = cd.getCollectionName(); 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 cccc0cba005..2f72b3b1adc 100644 --- a/solr/core/src/java/org/apache/solr/core/SolrCore.java +++ b/solr/core/src/java/org/apache/solr/core/SolrCore.java @@ -154,6 +154,7 @@ import org.apache.solr.search.facet.FacetParserFactory; import org.apache.solr.search.stats.LocalStatsCache; import org.apache.solr.search.stats.StatsCache; +import org.apache.solr.servlet.CoordinatorHttpSolrCall; import org.apache.solr.update.DefaultSolrCoreState; import org.apache.solr.update.DirectUpdateHandler2; import org.apache.solr.update.IndexFingerprint; @@ -1053,7 +1054,9 @@ public CoreContainer getCoreContainer() { return coreContainer; } - SolrCore(CoreContainer coreContainer, CoreDescriptor cd, ConfigSet configSet) { + + //TODO was protected. a proper way will likely be creating a SolrCoreProxy class + public SolrCore(CoreContainer coreContainer, CoreDescriptor cd, ConfigSet configSet) { this(coreContainer, cd, configSet, null, null, null, null, false); } @@ -1176,7 +1179,7 @@ private SolrCore( initSearcher(prev); // Initialize the RestManager - restManager = initRestManager(); + restManager = isSynthetic() ? new RestManager() : initRestManager(); // Finally tell anyone who wants to know resourceLoader.inform(resourceLoader); @@ -1203,7 +1206,9 @@ private SolrCore( // searcher! seedVersionBuckets(); - bufferUpdatesIfConstructing(coreDescriptor); + if (!isSynthetic()) { + bufferUpdatesIfConstructing(coreDescriptor); + } this.ruleExpiryLock = new ReentrantLock(); this.snapshotDelLock = new ReentrantLock(); @@ -3096,6 +3101,10 @@ public void fetchLatestSchema() { setLatestSchema(schema); } + public final boolean isSynthetic() { + return coreDescriptor.getCollectionName() != null && coreDescriptor.getCollectionName().startsWith(CoordinatorHttpSolrCall.SYNTHETIC_COLL_PREFIX); + } + public interface RawWriter { default String getContentType() { return BinaryResponseParser.BINARY_CONTENT_TYPE; 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 a2759b3edb8..486999d2ef8 100644 --- a/solr/core/src/java/org/apache/solr/servlet/CoordinatorHttpSolrCall.java +++ b/solr/core/src/java/org/apache/solr/servlet/CoordinatorHttpSolrCall.java @@ -89,8 +89,8 @@ public static SolrCore getCore( ZkStateReader zkStateReader = solrCall.cores.getZkController().getZkStateReader(); ClusterState clusterState = zkStateReader.getClusterState(); DocCollection coll = clusterState.getCollectionOrNull(collectionName, true); - SolrCore core = null; - if (coll != null) { + + synchronized (CoordinatorHttpSolrCall.class) { String confName = coll.getConfigName(); String syntheticCollectionName = getSyntheticCollectionName(confName); @@ -113,118 +113,9 @@ public static SolrCore getCore( //after this point the sync core should be available in the container. Double check if (coreContainer.getCore(syntheticCore.getName()) != null) { - factory.collectionVsCoreNameMapping.put(collectionName, core.getName()); + factory.collectionVsCoreNameMapping.put(collectionName, syntheticCore.getName()); return syntheticCore; } - - - 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); - } - - addReplica(syntheticCollectionName, solrCall.cores); - } - - // 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 -> { - for (Replica nodeNameSyntheticReplica : - docCollection.getReplicas(solrCall.cores.getZkController().getNodeName())) { - 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; } @@ -308,7 +199,9 @@ protected void init() throws Exception { 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, "_"); From 49cfa7bffe7ae952214bb1e67f517628a78afa05 Mon Sep 17 00:00:00 2001 From: patsonluk Date: Mon, 26 Feb 2024 09:24:29 -0800 Subject: [PATCH 04/20] ./gradlew tidy --- .../org/apache/solr/cloud/ZkConfigSetService.java | 5 +++-- .../org/apache/solr/core/ConfigSetService.java | 8 ++++++-- .../java/org/apache/solr/core/CoreContainer.java | 2 +- .../src/java/org/apache/solr/core/SolrCore.java | 8 +++++--- .../solr/servlet/CoordinatorHttpSolrCall.java | 15 ++++++++------- 5 files changed, 23 insertions(+), 15 deletions(-) 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 86a088ecef7..d29d7469e36 100644 --- a/solr/core/src/java/org/apache/solr/cloud/ZkConfigSetService.java +++ b/solr/core/src/java/org/apache/solr/cloud/ZkConfigSetService.java @@ -67,9 +67,10 @@ public ZkConfigSetService(SolrZkClient zkClient) { } @Override - protected SolrResourceLoader createConfigSetResourceLoader(CoreDescriptor cd, String configSetName) { + protected SolrResourceLoader createConfigSetResourceLoader( + CoreDescriptor cd, String configSetName) { return new ZkSolrResourceLoader( - cd.getInstanceDir(), configSetName, parentLoader.getClassLoader(), zkController); + cd.getInstanceDir(), configSetName, parentLoader.getClassLoader(), zkController); } @Override diff --git a/solr/core/src/java/org/apache/solr/core/ConfigSetService.java b/solr/core/src/java/org/apache/solr/core/ConfigSetService.java index b767d77c685..c1b2482090b 100644 --- a/solr/core/src/java/org/apache/solr/core/ConfigSetService.java +++ b/solr/core/src/java/org/apache/solr/core/ConfigSetService.java @@ -256,7 +256,10 @@ public final ConfigSet loadConfigSet(CoreDescriptor dcore) { */ public final ConfigSet loadConfigSet(CoreDescriptor dcore, String configSetName) { - SolrResourceLoader coreLoader = configSetName != null ? createConfigSetResourceLoader(dcore, configSetName) : createCoreResourceLoader(dcore); + SolrResourceLoader coreLoader = + configSetName != null + ? createConfigSetResourceLoader(dcore, configSetName) + : createCoreResourceLoader(dcore); try { // ConfigSet properties are loaded from ConfigSetProperties.DEFAULT_FILENAME file. @@ -395,7 +398,8 @@ protected NamedList loadConfigSetFlags(SolrResourceLoader loader) throws */ protected abstract SolrResourceLoader createCoreResourceLoader(CoreDescriptor cd); - protected SolrResourceLoader createConfigSetResourceLoader(CoreDescriptor cd, String configSetName) { + protected SolrResourceLoader createConfigSetResourceLoader( + CoreDescriptor cd, String configSetName) { throw new UnsupportedOperationException("Not supported"); } /** diff --git a/solr/core/src/java/org/apache/solr/core/CoreContainer.java b/solr/core/src/java/org/apache/solr/core/CoreContainer.java index 4c44f29a890..dcb251ef7b6 100644 --- a/solr/core/src/java/org/apache/solr/core/CoreContainer.java +++ b/solr/core/src/java/org/apache/solr/core/CoreContainer.java @@ -1433,7 +1433,7 @@ public CoresLocator getCoresLocator() { return coresLocator; } - //TODO was protected, should go with new class approach + // TODO was protected, should go with new class approach public SolrCore registerCore( CoreDescriptor cd, SolrCore core, boolean registerInZk, boolean skipRecovery) { if (core == null) { 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 2f72b3b1adc..095c1d5c3f1 100644 --- a/solr/core/src/java/org/apache/solr/core/SolrCore.java +++ b/solr/core/src/java/org/apache/solr/core/SolrCore.java @@ -1054,8 +1054,7 @@ public CoreContainer getCoreContainer() { return coreContainer; } - - //TODO was protected. a proper way will likely be creating a SolrCoreProxy class + // TODO was protected. a proper way will likely be creating a SolrCoreProxy class public SolrCore(CoreContainer coreContainer, CoreDescriptor cd, ConfigSet configSet) { this(coreContainer, cd, configSet, null, null, null, null, false); } @@ -3102,7 +3101,10 @@ public void fetchLatestSchema() { } public final boolean isSynthetic() { - return coreDescriptor.getCollectionName() != null && coreDescriptor.getCollectionName().startsWith(CoordinatorHttpSolrCall.SYNTHETIC_COLL_PREFIX); + return coreDescriptor.getCollectionName() != null + && coreDescriptor + .getCollectionName() + .startsWith(CoordinatorHttpSolrCall.SYNTHETIC_COLL_PREFIX); } public interface RawWriter { 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 486999d2ef8..bdf75f19b62 100644 --- a/solr/core/src/java/org/apache/solr/servlet/CoordinatorHttpSolrCall.java +++ b/solr/core/src/java/org/apache/solr/servlet/CoordinatorHttpSolrCall.java @@ -20,11 +20,9 @@ import java.lang.invoke.MethodHandles; import java.nio.file.Paths; import java.util.HashMap; -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; @@ -99,19 +97,22 @@ public static SolrCore getCore( coreProps.put(CoreAdminParams.CORE_NODE_NAME, coreContainer.getHostName()); coreProps.put(CoreAdminParams.COLLECTION, syntheticCollectionName); - CoreDescriptor syntheticCoreDescriptor = new CoreDescriptor( + CoreDescriptor syntheticCoreDescriptor = + new CoreDescriptor( collectionName, Paths.get(coreContainer.getSolrHome() + "/" + collectionName), - coreProps, coreContainer.getContainerProperties(), coreContainer.getZkController()); + coreProps, + coreContainer.getContainerProperties(), + coreContainer.getZkController()); - - ConfigSet coreConfig = coreContainer.getConfigSetService().loadConfigSet(syntheticCoreDescriptor, confName); + ConfigSet coreConfig = + coreContainer.getConfigSetService().loadConfigSet(syntheticCoreDescriptor, confName); syntheticCoreDescriptor.setConfigSetTrusted(coreConfig.isTrusted()); SolrCore syntheticCore = new SolrCore(coreContainer, syntheticCoreDescriptor, coreConfig); coreContainer.registerCore(syntheticCoreDescriptor, syntheticCore, false, false); - //after this point the sync core should be available in the container. Double check + // after this point the sync core should be available in the container. Double check if (coreContainer.getCore(syntheticCore.getName()) != null) { factory.collectionVsCoreNameMapping.put(collectionName, syntheticCore.getName()); return syntheticCore; From 5de2a27caeba7d15eb33fa8772776e3cd0a5fcee Mon Sep 17 00:00:00 2001 From: patsonluk Date: Mon, 26 Feb 2024 09:29:45 -0800 Subject: [PATCH 05/20] Comment out portion that checks for synthetic collection/core existence --- .../solr/search/TestCoordinatorRole.java | 36 +++++++++---------- 1 file changed, 18 insertions(+), 18 deletions(-) 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 a0b1d1bef99..49214e83446 100644 --- a/solr/core/src/test/org/apache/solr/search/TestCoordinatorRole.java +++ b/solr/core/src/test/org/apache/solr/search/TestCoordinatorRole.java @@ -69,7 +69,7 @@ public void testSimple() throws Exception { try { CloudSolrClient client = cluster.getSolrClient(); String COLLECTION_NAME = "test_coll"; - String SYNTHETIC_COLLECTION = CoordinatorHttpSolrCall.SYNTHETIC_COLL_PREFIX + "conf"; + //String SYNTHETIC_COLLECTION = CoordinatorHttpSolrCall.SYNTHETIC_COLL_PREFIX + "conf"; CollectionAdminRequest.createCollection(COLLECTION_NAME, "conf", 2, 2) .process(cluster.getSolrClient()); cluster.waitForActiveCollection(COLLECTION_NAME, 2, 4); @@ -98,14 +98,14 @@ public void testSimple() throws Exception { assertEquals(10, rslt.getResults().size()); - 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()); +// 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()); } finally { cluster.shutdown(); } @@ -117,7 +117,7 @@ public void testMultiCollectionMultiNode() throws Exception { try { CloudSolrClient client = cluster.getSolrClient(); String COLLECTION_NAME = "test_coll"; - String SYNTHETIC_COLLECTION = CoordinatorHttpSolrCall.SYNTHETIC_COLL_PREFIX + "conf"; + //String SYNTHETIC_COLLECTION = CoordinatorHttpSolrCall.SYNTHETIC_COLL_PREFIX + "conf"; for (int j = 1; j <= 10; j++) { String collname = COLLECTION_NAME + "_" + j; CollectionAdminRequest.createCollection(collname, "conf", 2, 2) @@ -164,14 +164,14 @@ 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); +// 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(); } From 924391a67fa0dfc76548abac1d269e7dc2d2c1cb Mon Sep 17 00:00:00 2001 From: patsonluk Date: Mon, 26 Feb 2024 09:56:03 -0800 Subject: [PATCH 06/20] Fixes watches and test cases --- .../solr/servlet/CoordinatorHttpSolrCall.java | 94 +++++-------------- .../solr/search/TestCoordinatorRole.java | 46 ++++----- 2 files changed, 45 insertions(+), 95 deletions(-) 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 bdf75f19b62..7be262ebebb 100644 --- a/solr/core/src/java/org/apache/solr/servlet/CoordinatorHttpSolrCall.java +++ b/solr/core/src/java/org/apache/solr/servlet/CoordinatorHttpSolrCall.java @@ -26,26 +26,19 @@ 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.CoreAdminParams; -import org.apache.solr.common.params.SolrParams; -import org.apache.solr.common.util.Utils; import org.apache.solr.core.ConfigSet; import org.apache.solr.core.CoreContainer; import org.apache.solr.core.CoreDescriptor; import org.apache.solr.core.SolrCore; -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; @@ -80,9 +73,7 @@ public static SolrCore getCore( Factory factory, HttpSolrCall solrCall, String collectionName, boolean isPreferLeader) { String syntheticCoreName = factory.collectionVsCoreNameMapping.get(collectionName); if (syntheticCoreName != null) { - SolrCore syntheticCore = solrCall.cores.getCore(syntheticCoreName); - setMDCLoggingContext(collectionName); - return syntheticCore; + return solrCall.cores.getCore(syntheticCoreName); } else { ZkStateReader zkStateReader = solrCall.cores.getZkController().getZkStateReader(); ClusterState clusterState = zkStateReader.getClusterState(); @@ -115,6 +106,26 @@ public static SolrCore getCore( // after this point the sync core should be available in the container. Double check if (coreContainer.getCore(syntheticCore.getName()) != null) { factory.collectionVsCoreNameMapping.put(collectionName, syntheticCore.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: {}", syntheticCore.getName()); + } return syntheticCore; } } @@ -126,69 +137,6 @@ public static String getSyntheticCollectionName(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); - } - } - @Override protected void init() throws Exception { super.init(); 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 49214e83446..9b660db06d6 100644 --- a/solr/core/src/test/org/apache/solr/search/TestCoordinatorRole.java +++ b/solr/core/src/test/org/apache/solr/search/TestCoordinatorRole.java @@ -23,11 +23,9 @@ import java.lang.invoke.MethodHandles; 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; @@ -56,7 +54,6 @@ import org.apache.solr.common.util.SolrNamedThreadFactory; import org.apache.solr.core.NodeRoles; import org.apache.solr.embedded.JettySolrRunner; -import org.apache.solr.servlet.CoordinatorHttpSolrCall; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -69,7 +66,7 @@ public void testSimple() throws Exception { try { CloudSolrClient client = cluster.getSolrClient(); String COLLECTION_NAME = "test_coll"; - //String SYNTHETIC_COLLECTION = CoordinatorHttpSolrCall.SYNTHETIC_COLL_PREFIX + "conf"; + // String SYNTHETIC_COLLECTION = CoordinatorHttpSolrCall.SYNTHETIC_COLL_PREFIX + "conf"; CollectionAdminRequest.createCollection(COLLECTION_NAME, "conf", 2, 2) .process(cluster.getSolrClient()); cluster.waitForActiveCollection(COLLECTION_NAME, 2, 4); @@ -98,14 +95,16 @@ public void testSimple() throws Exception { assertEquals(10, rslt.getResults().size()); -// 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()); + // 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()); } finally { cluster.shutdown(); } @@ -117,7 +116,7 @@ public void testMultiCollectionMultiNode() throws Exception { try { CloudSolrClient client = cluster.getSolrClient(); String COLLECTION_NAME = "test_coll"; - //String SYNTHETIC_COLLECTION = CoordinatorHttpSolrCall.SYNTHETIC_COLL_PREFIX + "conf"; + // String SYNTHETIC_COLLECTION = CoordinatorHttpSolrCall.SYNTHETIC_COLL_PREFIX + "conf"; for (int j = 1; j <= 10; j++) { String collname = COLLECTION_NAME + "_" + j; CollectionAdminRequest.createCollection(collname, "conf", 2, 2) @@ -164,14 +163,17 @@ 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); + // 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(); } @@ -530,7 +532,7 @@ public void testWatch() throws Exception { // ensure querying throws exception assertExceptionThrownWithMessageContaining( SolrException.class, - List.of("Collection not found"), + List.of("Could not find collection"), () -> new QueryRequest(new SolrQuery("*:*")) .setPreferredNodes(List.of(coordinatorJetty.getNodeName())) From 6772c22170ee90a6093382e213fe355ae778baf3 Mon Sep 17 00:00:00 2001 From: patsonluk Date: Mon, 26 Feb 2024 11:19:13 -0800 Subject: [PATCH 07/20] Refactoring to avoid modifying existing method modifiers --- .../org/apache/solr/core/CoreContainer.java | 3 +- .../java/org/apache/solr/core/SolrCore.java | 31 +++++++------- .../org/apache/solr/core/SolrCoreProxy.java | 41 +++++++++++++++++++ .../solr/servlet/CoordinatorHttpSolrCall.java | 32 +++++---------- .../solr/search/TestCoordinatorRole.java | 2 +- 5 files changed, 69 insertions(+), 40 deletions(-) create mode 100644 solr/core/src/java/org/apache/solr/core/SolrCoreProxy.java diff --git a/solr/core/src/java/org/apache/solr/core/CoreContainer.java b/solr/core/src/java/org/apache/solr/core/CoreContainer.java index dcb251ef7b6..0fc202b0c5b 100644 --- a/solr/core/src/java/org/apache/solr/core/CoreContainer.java +++ b/solr/core/src/java/org/apache/solr/core/CoreContainer.java @@ -1433,8 +1433,7 @@ public CoresLocator getCoresLocator() { return coresLocator; } - // TODO was protected, should go with new class approach - public SolrCore registerCore( + protected SolrCore registerCore( CoreDescriptor cd, SolrCore core, boolean registerInZk, boolean skipRecovery) { if (core == null) { throw new RuntimeException("Can not register a null core."); 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 095c1d5c3f1..b2f79b4be84 100644 --- a/solr/core/src/java/org/apache/solr/core/SolrCore.java +++ b/solr/core/src/java/org/apache/solr/core/SolrCore.java @@ -154,7 +154,6 @@ import org.apache.solr.search.facet.FacetParserFactory; import org.apache.solr.search.stats.LocalStatsCache; import org.apache.solr.search.stats.StatsCache; -import org.apache.solr.servlet.CoordinatorHttpSolrCall; import org.apache.solr.update.DefaultSolrCoreState; import org.apache.solr.update.DirectUpdateHandler2; import org.apache.solr.update.IndexFingerprint; @@ -207,6 +206,7 @@ public class SolrCore implements SolrInfoBean, Closeable { private static final Logger slowLog = LoggerFactory.getLogger( MethodHandles.lookup().lookupClass().getName() + ".SlowRequest"); // nowarn + private final boolean isSynthetic; private String name; @@ -792,7 +792,8 @@ public SolrCore reload(ConfigSet coreConfig) throws IOException { updateHandler, solrDelPolicy, currentCore, - true); + true, + isSynthetic); // we open a new IndexWriter to pick up the latest config core.getUpdateHandler().getSolrCoreState().newIndexWriter(core, false); @@ -1054,9 +1055,13 @@ public CoreContainer getCoreContainer() { return coreContainer; } - // TODO was protected. a proper way will likely be creating a SolrCoreProxy class - public SolrCore(CoreContainer coreContainer, CoreDescriptor cd, ConfigSet configSet) { - this(coreContainer, cd, configSet, null, null, null, null, false); + protected SolrCore(CoreContainer coreContainer, CoreDescriptor cd, ConfigSet configSet) { + this(coreContainer, cd, configSet, false); + } + + protected SolrCore( + CoreContainer coreContainer, CoreDescriptor cd, ConfigSet configSet, boolean isSynthetic) { + this(coreContainer, cd, configSet, null, null, null, null, false, isSynthetic); } private SolrCore( @@ -1067,7 +1072,8 @@ private SolrCore( UpdateHandler updateHandler, IndexDeletionPolicyWrapper delPolicy, SolrCore prev, - boolean reload) { + boolean reload, + boolean isSynthetic) { // ensure that in unclean shutdown tests we still close this assert ObjectReleaseTracker.track(searcherExecutor); @@ -1177,8 +1183,10 @@ private SolrCore( initSearcher(prev); + this.isSynthetic = isSynthetic; + // Initialize the RestManager - restManager = isSynthetic() ? new RestManager() : initRestManager(); + restManager = isSynthetic ? new RestManager() : initRestManager(); // Finally tell anyone who wants to know resourceLoader.inform(resourceLoader); @@ -1205,7 +1213,7 @@ private SolrCore( // searcher! seedVersionBuckets(); - if (!isSynthetic()) { + if (!isSynthetic) { bufferUpdatesIfConstructing(coreDescriptor); } @@ -3100,13 +3108,6 @@ public void fetchLatestSchema() { setLatestSchema(schema); } - public final boolean isSynthetic() { - return coreDescriptor.getCollectionName() != null - && coreDescriptor - .getCollectionName() - .startsWith(CoordinatorHttpSolrCall.SYNTHETIC_COLL_PREFIX); - } - public interface RawWriter { default String getContentType() { return BinaryResponseParser.BINARY_CONTENT_TYPE; diff --git a/solr/core/src/java/org/apache/solr/core/SolrCoreProxy.java b/solr/core/src/java/org/apache/solr/core/SolrCoreProxy.java new file mode 100644 index 00000000000..bd01f561fb0 --- /dev/null +++ b/solr/core/src/java/org/apache/solr/core/SolrCoreProxy.java @@ -0,0 +1,41 @@ +package org.apache.solr.core; + +import java.nio.file.Paths; +import java.util.HashMap; +import java.util.Map; +import org.apache.solr.common.params.CoreAdminParams; + +public class SolrCoreProxy extends SolrCore { + public SolrCoreProxy(CoreContainer coreContainer, CoreDescriptor cd, ConfigSet configSet) { + super(coreContainer, cd, configSet, true); + } + + public static SolrCoreProxy createAndRegisterProxy( + CoreContainer coreContainer, + String syntheticCollectionName, + String configSetName, + String todo) { + Map coreProps = new HashMap<>(); + coreProps.put(CoreAdminParams.CORE_NODE_NAME, coreContainer.getHostName()); + coreProps.put(CoreAdminParams.COLLECTION, syntheticCollectionName); + + CoreDescriptor syntheticCoreDescriptor = + new CoreDescriptor( + syntheticCollectionName, + // todo, + Paths.get(coreContainer.getSolrHome() + "/" + syntheticCollectionName), + // Paths.get(coreContainer.getSolrHome() + "/" + todo), + coreProps, + coreContainer.getContainerProperties(), + coreContainer.getZkController()); + + ConfigSet coreConfig = + coreContainer.getConfigSetService().loadConfigSet(syntheticCoreDescriptor, configSetName); + syntheticCoreDescriptor.setConfigSetTrusted(coreConfig.isTrusted()); + SolrCoreProxy syntheticCore = + new SolrCoreProxy(coreContainer, syntheticCoreDescriptor, coreConfig); + coreContainer.registerCore(syntheticCoreDescriptor, syntheticCore, false, false); + + return syntheticCore; + } +} 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 7be262ebebb..cf3baf7e22a 100644 --- a/solr/core/src/java/org/apache/solr/servlet/CoordinatorHttpSolrCall.java +++ b/solr/core/src/java/org/apache/solr/servlet/CoordinatorHttpSolrCall.java @@ -18,8 +18,6 @@ package org.apache.solr.servlet; import java.lang.invoke.MethodHandles; -import java.nio.file.Paths; -import java.util.HashMap; import java.util.Map; import java.util.Properties; import java.util.concurrent.ConcurrentHashMap; @@ -32,11 +30,10 @@ 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.CoreAdminParams; -import org.apache.solr.core.ConfigSet; import org.apache.solr.core.CoreContainer; import org.apache.solr.core.CoreDescriptor; import org.apache.solr.core.SolrCore; +import org.apache.solr.core.SolrCoreProxy; import org.apache.solr.request.DelegatingSolrQueryRequest; import org.apache.solr.request.SolrQueryRequest; import org.slf4j.Logger; @@ -79,29 +76,20 @@ public static SolrCore getCore( ClusterState clusterState = zkStateReader.getClusterState(); DocCollection coll = clusterState.getCollectionOrNull(collectionName, true); + 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; + } synchronized (CoordinatorHttpSolrCall.class) { String confName = coll.getConfigName(); String syntheticCollectionName = getSyntheticCollectionName(confName); CoreContainer coreContainer = solrCall.cores; - Map coreProps = new HashMap<>(); - coreProps.put(CoreAdminParams.CORE_NODE_NAME, coreContainer.getHostName()); - coreProps.put(CoreAdminParams.COLLECTION, syntheticCollectionName); - - CoreDescriptor syntheticCoreDescriptor = - new CoreDescriptor( - collectionName, - Paths.get(coreContainer.getSolrHome() + "/" + collectionName), - coreProps, - coreContainer.getContainerProperties(), - coreContainer.getZkController()); - - ConfigSet coreConfig = - coreContainer.getConfigSetService().loadConfigSet(syntheticCoreDescriptor, confName); - syntheticCoreDescriptor.setConfigSetTrusted(coreConfig.isTrusted()); - SolrCore syntheticCore = new SolrCore(coreContainer, syntheticCoreDescriptor, coreConfig); - - coreContainer.registerCore(syntheticCoreDescriptor, syntheticCore, false, false); + SolrCoreProxy syntheticCore = + SolrCoreProxy.createAndRegisterProxy( + coreContainer, syntheticCollectionName, coll.getConfigName(), collectionName); // after this point the sync core should be available in the container. Double check if (coreContainer.getCore(syntheticCore.getName()) != null) { 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 9b660db06d6..26d115bf7bd 100644 --- a/solr/core/src/test/org/apache/solr/search/TestCoordinatorRole.java +++ b/solr/core/src/test/org/apache/solr/search/TestCoordinatorRole.java @@ -532,7 +532,7 @@ public void testWatch() throws Exception { // ensure querying throws exception assertExceptionThrownWithMessageContaining( SolrException.class, - List.of("Could not find collection"), + List.of("Collection not found"), () -> new QueryRequest(new SolrQuery("*:*")) .setPreferredNodes(List.of(coordinatorJetty.getNodeName())) From fa08d7af31d5e56560df5b8a48ca753c0776417a Mon Sep 17 00:00:00 2001 From: patsonluk Date: Mon, 26 Feb 2024 11:23:10 -0800 Subject: [PATCH 08/20] Cleanup --- solr/core/src/java/org/apache/solr/core/CoreContainer.java | 2 +- solr/core/src/java/org/apache/solr/core/SolrCoreProxy.java | 7 +------ .../org/apache/solr/servlet/CoordinatorHttpSolrCall.java | 2 +- 3 files changed, 3 insertions(+), 8 deletions(-) diff --git a/solr/core/src/java/org/apache/solr/core/CoreContainer.java b/solr/core/src/java/org/apache/solr/core/CoreContainer.java index 0fc202b0c5b..2aae7e733b9 100644 --- a/solr/core/src/java/org/apache/solr/core/CoreContainer.java +++ b/solr/core/src/java/org/apache/solr/core/CoreContainer.java @@ -1644,7 +1644,7 @@ public AllowListUrlChecker getAllowListUrlChecker() { * @return the newly created core */ @SuppressWarnings("resource") - public SolrCore createFromDescriptor( + private SolrCore createFromDescriptor( CoreDescriptor dcore, boolean publishState, boolean newCollection) { if (isShutDown) { diff --git a/solr/core/src/java/org/apache/solr/core/SolrCoreProxy.java b/solr/core/src/java/org/apache/solr/core/SolrCoreProxy.java index bd01f561fb0..bff8a1f4b80 100644 --- a/solr/core/src/java/org/apache/solr/core/SolrCoreProxy.java +++ b/solr/core/src/java/org/apache/solr/core/SolrCoreProxy.java @@ -11,10 +11,7 @@ public SolrCoreProxy(CoreContainer coreContainer, CoreDescriptor cd, ConfigSet c } public static SolrCoreProxy createAndRegisterProxy( - CoreContainer coreContainer, - String syntheticCollectionName, - String configSetName, - String todo) { + CoreContainer coreContainer, String syntheticCollectionName, String configSetName) { Map coreProps = new HashMap<>(); coreProps.put(CoreAdminParams.CORE_NODE_NAME, coreContainer.getHostName()); coreProps.put(CoreAdminParams.COLLECTION, syntheticCollectionName); @@ -22,9 +19,7 @@ public static SolrCoreProxy createAndRegisterProxy( CoreDescriptor syntheticCoreDescriptor = new CoreDescriptor( syntheticCollectionName, - // todo, Paths.get(coreContainer.getSolrHome() + "/" + syntheticCollectionName), - // Paths.get(coreContainer.getSolrHome() + "/" + todo), coreProps, coreContainer.getContainerProperties(), coreContainer.getZkController()); 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 cf3baf7e22a..b0aa18e7e9b 100644 --- a/solr/core/src/java/org/apache/solr/servlet/CoordinatorHttpSolrCall.java +++ b/solr/core/src/java/org/apache/solr/servlet/CoordinatorHttpSolrCall.java @@ -89,7 +89,7 @@ public static SolrCore getCore( CoreContainer coreContainer = solrCall.cores; SolrCoreProxy syntheticCore = SolrCoreProxy.createAndRegisterProxy( - coreContainer, syntheticCollectionName, coll.getConfigName(), collectionName); + coreContainer, syntheticCollectionName, coll.getConfigName()); // after this point the sync core should be available in the container. Double check if (coreContainer.getCore(syntheticCore.getName()) != null) { From 0d2b744204cd263065cfbf80127266fa43b4a69e Mon Sep 17 00:00:00 2001 From: patsonluk Date: Wed, 17 Apr 2024 17:30:58 -0700 Subject: [PATCH 09/20] Some refactoring to minimize changes to SolrCore --- .../java/org/apache/solr/core/SolrCore.java | 26 +++++-------------- ...rCoreProxy.java => SyntheticSolrCore.java} | 25 +++++++++++++----- .../solr/servlet/CoordinatorHttpSolrCall.java | 6 ++--- 3 files changed, 29 insertions(+), 28 deletions(-) rename solr/core/src/java/org/apache/solr/core/{SolrCoreProxy.java => SyntheticSolrCore.java} (59%) 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 b2f79b4be84..351aafbf729 100644 --- a/solr/core/src/java/org/apache/solr/core/SolrCore.java +++ b/solr/core/src/java/org/apache/solr/core/SolrCore.java @@ -206,7 +206,6 @@ public class SolrCore implements SolrInfoBean, Closeable { private static final Logger slowLog = LoggerFactory.getLogger( MethodHandles.lookup().lookupClass().getName() + ".SlowRequest"); // nowarn - private final boolean isSynthetic; private String name; @@ -792,8 +791,7 @@ public SolrCore reload(ConfigSet coreConfig) throws IOException { updateHandler, solrDelPolicy, currentCore, - true, - isSynthetic); + true); // we open a new IndexWriter to pick up the latest config core.getUpdateHandler().getSolrCoreState().newIndexWriter(core, false); @@ -1055,13 +1053,8 @@ public CoreContainer getCoreContainer() { return coreContainer; } - protected SolrCore(CoreContainer coreContainer, CoreDescriptor cd, ConfigSet configSet) { - this(coreContainer, cd, configSet, false); - } - - protected SolrCore( - CoreContainer coreContainer, CoreDescriptor cd, ConfigSet configSet, boolean isSynthetic) { - this(coreContainer, cd, configSet, null, null, null, null, false, isSynthetic); + SolrCore(CoreContainer coreContainer, CoreDescriptor cd, ConfigSet configSet) { + this(coreContainer, cd, configSet, null, null, null, null, false); } private SolrCore( @@ -1072,8 +1065,7 @@ private SolrCore( UpdateHandler updateHandler, IndexDeletionPolicyWrapper delPolicy, SolrCore prev, - boolean reload, - boolean isSynthetic) { + boolean reload) { // ensure that in unclean shutdown tests we still close this assert ObjectReleaseTracker.track(searcherExecutor); @@ -1183,10 +1175,8 @@ private SolrCore( initSearcher(prev); - this.isSynthetic = isSynthetic; - // Initialize the RestManager - restManager = isSynthetic ? new RestManager() : initRestManager(); + restManager = initRestManager(); // Finally tell anyone who wants to know resourceLoader.inform(resourceLoader); @@ -1213,9 +1203,7 @@ private SolrCore( // searcher! seedVersionBuckets(); - if (!isSynthetic) { - bufferUpdatesIfConstructing(coreDescriptor); - } + bufferUpdatesIfConstructing(coreDescriptor); this.ruleExpiryLock = new ReentrantLock(); this.snapshotDelLock = new ReentrantLock(); @@ -1267,7 +1255,7 @@ public void seedVersionBuckets() { } /** 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/SolrCoreProxy.java b/solr/core/src/java/org/apache/solr/core/SyntheticSolrCore.java similarity index 59% rename from solr/core/src/java/org/apache/solr/core/SolrCoreProxy.java rename to solr/core/src/java/org/apache/solr/core/SyntheticSolrCore.java index bff8a1f4b80..70aa6f524ab 100644 --- a/solr/core/src/java/org/apache/solr/core/SolrCoreProxy.java +++ b/solr/core/src/java/org/apache/solr/core/SyntheticSolrCore.java @@ -3,14 +3,17 @@ import java.nio.file.Paths; import java.util.HashMap; import java.util.Map; + +import org.apache.solr.common.SolrException; import org.apache.solr.common.params.CoreAdminParams; +import org.apache.solr.rest.RestManager; -public class SolrCoreProxy extends SolrCore { - public SolrCoreProxy(CoreContainer coreContainer, CoreDescriptor cd, ConfigSet configSet) { - super(coreContainer, cd, configSet, true); +public class SyntheticSolrCore extends SolrCore { + public SyntheticSolrCore(CoreContainer coreContainer, CoreDescriptor cd, ConfigSet configSet) { + super(coreContainer, cd, configSet); } - public static SolrCoreProxy createAndRegisterProxy( + public static SyntheticSolrCore createAndRegisterCore( CoreContainer coreContainer, String syntheticCollectionName, String configSetName) { Map coreProps = new HashMap<>(); coreProps.put(CoreAdminParams.CORE_NODE_NAME, coreContainer.getHostName()); @@ -27,10 +30,20 @@ public static SolrCoreProxy createAndRegisterProxy( ConfigSet coreConfig = coreContainer.getConfigSetService().loadConfigSet(syntheticCoreDescriptor, configSetName); syntheticCoreDescriptor.setConfigSetTrusted(coreConfig.isTrusted()); - SolrCoreProxy syntheticCore = - new SolrCoreProxy(coreContainer, syntheticCoreDescriptor, coreConfig); + 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 { + return new RestManager(); //TODO explain why we cannot use the super class init routine + } } 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 b0aa18e7e9b..6079b8b009f 100644 --- a/solr/core/src/java/org/apache/solr/servlet/CoordinatorHttpSolrCall.java +++ b/solr/core/src/java/org/apache/solr/servlet/CoordinatorHttpSolrCall.java @@ -33,7 +33,7 @@ import org.apache.solr.core.CoreContainer; import org.apache.solr.core.CoreDescriptor; import org.apache.solr.core.SolrCore; -import org.apache.solr.core.SolrCoreProxy; +import org.apache.solr.core.SyntheticSolrCore; import org.apache.solr.request.DelegatingSolrQueryRequest; import org.apache.solr.request.SolrQueryRequest; import org.slf4j.Logger; @@ -87,8 +87,8 @@ public static SolrCore getCore( String syntheticCollectionName = getSyntheticCollectionName(confName); CoreContainer coreContainer = solrCall.cores; - SolrCoreProxy syntheticCore = - SolrCoreProxy.createAndRegisterProxy( + SyntheticSolrCore syntheticCore = + SyntheticSolrCore.createAndRegisterCore( coreContainer, syntheticCollectionName, coll.getConfigName()); // after this point the sync core should be available in the container. Double check From a1582f65e66884476761040fe43612c08b79c9c0 Mon Sep 17 00:00:00 2001 From: patsonluk Date: Wed, 17 Apr 2024 18:26:20 -0700 Subject: [PATCH 10/20] javadoc --- .../org/apache/solr/core/SyntheticSolrCore.java | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/solr/core/src/java/org/apache/solr/core/SyntheticSolrCore.java b/solr/core/src/java/org/apache/solr/core/SyntheticSolrCore.java index 70aa6f524ab..b9af129ec1d 100644 --- a/solr/core/src/java/org/apache/solr/core/SyntheticSolrCore.java +++ b/solr/core/src/java/org/apache/solr/core/SyntheticSolrCore.java @@ -8,6 +8,14 @@ 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); @@ -39,11 +47,14 @@ public static SyntheticSolrCore createAndRegisterCore( @Override protected void bufferUpdatesIfConstructing(CoreDescriptor coreDescriptor) { - //no updates to SyntheticSolrCore + // no updates to SyntheticSolrCore } @Override protected RestManager initRestManager() throws SolrException { - return new RestManager(); //TODO explain why we cannot use the super class init routine + // 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(); } } From bff8168ca8ac62af2b7762260da9cb1fe721ab27 Mon Sep 17 00:00:00 2001 From: patsonluk Date: Wed, 17 Apr 2024 18:34:30 -0700 Subject: [PATCH 11/20] ./gradlew tidy --- .../src/java/org/apache/solr/core/SyntheticSolrCore.java | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/solr/core/src/java/org/apache/solr/core/SyntheticSolrCore.java b/solr/core/src/java/org/apache/solr/core/SyntheticSolrCore.java index b9af129ec1d..d77cc013c56 100644 --- a/solr/core/src/java/org/apache/solr/core/SyntheticSolrCore.java +++ b/solr/core/src/java/org/apache/solr/core/SyntheticSolrCore.java @@ -3,7 +3,6 @@ import java.nio.file.Paths; import java.util.HashMap; import java.util.Map; - import org.apache.solr.common.SolrException; import org.apache.solr.common.params.CoreAdminParams; import org.apache.solr.rest.RestManager; @@ -11,8 +10,9 @@ /** * 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. + *

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 */ @@ -52,7 +52,8 @@ protected void bufferUpdatesIfConstructing(CoreDescriptor coreDescriptor) { @Override protected RestManager initRestManager() throws SolrException { - // returns an initialized RestManager. As init routines requires reading configname of the core's collection from ZK + // 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(); From 28e5669cbee5497858bbd4b235c0e944cb68c2c7 Mon Sep 17 00:00:00 2001 From: patsonluk Date: Thu, 18 Apr 2024 14:32:21 -0700 Subject: [PATCH 12/20] code cleanup --- .../solr/search/TestCoordinatorRole.java | 25 ------------------- 1 file changed, 25 deletions(-) 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 26d115bf7bd..6107ccec44a 100644 --- a/solr/core/src/test/org/apache/solr/search/TestCoordinatorRole.java +++ b/solr/core/src/test/org/apache/solr/search/TestCoordinatorRole.java @@ -66,7 +66,6 @@ public void testSimple() throws Exception { try { CloudSolrClient client = cluster.getSolrClient(); String COLLECTION_NAME = "test_coll"; - // String SYNTHETIC_COLLECTION = CoordinatorHttpSolrCall.SYNTHETIC_COLL_PREFIX + "conf"; CollectionAdminRequest.createCollection(COLLECTION_NAME, "conf", 2, 2) .process(cluster.getSolrClient()); cluster.waitForActiveCollection(COLLECTION_NAME, 2, 4); @@ -94,17 +93,6 @@ public void testSimple() throws Exception { .process(client, COLLECTION_NAME); assertEquals(10, rslt.getResults().size()); - - // 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()); } finally { cluster.shutdown(); } @@ -116,7 +104,6 @@ public void testMultiCollectionMultiNode() throws Exception { try { CloudSolrClient client = cluster.getSolrClient(); String COLLECTION_NAME = "test_coll"; - // String SYNTHETIC_COLLECTION = CoordinatorHttpSolrCall.SYNTHETIC_COLL_PREFIX + "conf"; for (int j = 1; j <= 10; j++) { String collname = COLLECTION_NAME + "_" + j; CollectionAdminRequest.createCollection(collname, "conf", 2, 2) @@ -162,18 +149,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(); } From dd259c00d078ed688792507e9d53b7ff35555584 Mon Sep 17 00:00:00 2001 From: patsonluk Date: Thu, 18 Apr 2024 14:56:24 -0700 Subject: [PATCH 13/20] code cleanup --- .../apache/solr/cloud/ZkConfigSetService.java | 3 +- .../apache/solr/core/ConfigSetService.java | 32 +++++++++++++------ 2 files changed, 24 insertions(+), 11 deletions(-) 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 d29d7469e36..fcaead583d2 100644 --- a/solr/core/src/java/org/apache/solr/cloud/ZkConfigSetService.java +++ b/solr/core/src/java/org/apache/solr/cloud/ZkConfigSetService.java @@ -67,8 +67,7 @@ public ZkConfigSetService(SolrZkClient zkClient) { } @Override - protected SolrResourceLoader createConfigSetResourceLoader( - CoreDescriptor cd, String configSetName) { + protected SolrResourceLoader createCoreResourceLoader(CoreDescriptor cd, String configSetName) { return new ZkSolrResourceLoader( cd.getInstanceDir(), configSetName, parentLoader.getClassLoader(), zkController); } diff --git a/solr/core/src/java/org/apache/solr/core/ConfigSetService.java b/solr/core/src/java/org/apache/solr/core/ConfigSetService.java index c1b2482090b..c6d333f7d21 100644 --- a/solr/core/src/java/org/apache/solr/core/ConfigSetService.java +++ b/solr/core/src/java/org/apache/solr/core/ConfigSetService.java @@ -244,22 +244,26 @@ public boolean isConfigSetTrusted(SolrResourceLoader coreLoader) throws IOExcept return (flags == null || flags.get("trusted") == null || flags.getBooleanArg("trusted")); } + /** + * Load the ConfigSet for a core + * + * @param dcore the core's CoreDescriptor + * @return a ConfigSet + */ public final ConfigSet loadConfigSet(CoreDescriptor dcore) { return loadConfigSet(dcore, null); } /** - * Load the ConfigSet for a core + * Load the ConfigSet for a core with an explicit config set name * * @param dcore the core's CoreDescriptor + * @param configSetName an optional and explicit config set name * @return a ConfigSet */ - public final ConfigSet loadConfigSet(CoreDescriptor dcore, String configSetName) { + final ConfigSet loadConfigSet(CoreDescriptor dcore, String configSetName) { - SolrResourceLoader coreLoader = - configSetName != null - ? createConfigSetResourceLoader(dcore, configSetName) - : createCoreResourceLoader(dcore); + SolrResourceLoader coreLoader = createCoreResourceLoader(dcore, configSetName); try { // ConfigSet properties are loaded from ConfigSetProperties.DEFAULT_FILENAME file. @@ -398,9 +402,19 @@ protected NamedList loadConfigSetFlags(SolrResourceLoader loader) throws */ protected abstract SolrResourceLoader createCoreResourceLoader(CoreDescriptor cd); - protected SolrResourceLoader createConfigSetResourceLoader( - CoreDescriptor cd, String configSetName) { - throw new UnsupportedOperationException("Not supported"); + /** + * Create a SolrResourceLoader for a core with the provided configSetName. + * + *

By default, this will just call {@link + * ConfigSetService#createConfigSetService(CoreContainer)}. Child implementation might override + * this to make use of the configSetName directly + * + * @param cd the core's CoreDescriptor + * @param configSetName an optional config set name + * @return a SolrResourceLoader + */ + protected SolrResourceLoader createCoreResourceLoader(CoreDescriptor cd, String configSetName) { + return createCoreResourceLoader(cd); } /** * Return a name for the ConfigSet for a core to be used for printing/diagnostic purposes. From b8a15e739bf179ab009ae4a96708a9743affc0f2 Mon Sep 17 00:00:00 2001 From: patsonluk Date: Thu, 18 Apr 2024 16:27:49 -0700 Subject: [PATCH 14/20] Fix resource loader issue if configSetName is null --- .../java/org/apache/solr/cloud/ZkConfigSetService.java | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) 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 fcaead583d2..ccac0d87e5b 100644 --- a/solr/core/src/java/org/apache/solr/cloud/ZkConfigSetService.java +++ b/solr/core/src/java/org/apache/solr/cloud/ZkConfigSetService.java @@ -66,10 +66,15 @@ public ZkConfigSetService(SolrZkClient zkClient) { this.zkClient = zkClient; } + /** + * Do not perform Zk operations if configSetName is provided. + * + *

This is only used by {@link org.apache.solr.core.SyntheticSolrCore}, which is not registered with Zookeeper + */ @Override protected SolrResourceLoader createCoreResourceLoader(CoreDescriptor cd, String configSetName) { - return new ZkSolrResourceLoader( - cd.getInstanceDir(), configSetName, parentLoader.getClassLoader(), zkController); + return configSetName != null ? new ZkSolrResourceLoader( + cd.getInstanceDir(), configSetName, parentLoader.getClassLoader(), zkController) : createCoreResourceLoader(cd); } @Override From bb11583a7663d01b1fa5096bcaab3c3db1f227de Mon Sep 17 00:00:00 2001 From: patsonluk Date: Thu, 18 Apr 2024 17:11:01 -0700 Subject: [PATCH 15/20] ./gradlew tidy --- .../java/org/apache/solr/cloud/ZkConfigSetService.java | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) 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 ccac0d87e5b..07f310c696e 100644 --- a/solr/core/src/java/org/apache/solr/cloud/ZkConfigSetService.java +++ b/solr/core/src/java/org/apache/solr/cloud/ZkConfigSetService.java @@ -69,12 +69,15 @@ public ZkConfigSetService(SolrZkClient zkClient) { /** * Do not perform Zk operations if configSetName is provided. * - *

This is only used by {@link org.apache.solr.core.SyntheticSolrCore}, which is not registered with Zookeeper + *

This is only used by {@link org.apache.solr.core.SyntheticSolrCore}, which is not registered + * with Zookeeper */ @Override protected SolrResourceLoader createCoreResourceLoader(CoreDescriptor cd, String configSetName) { - return configSetName != null ? new ZkSolrResourceLoader( - cd.getInstanceDir(), configSetName, parentLoader.getClassLoader(), zkController) : createCoreResourceLoader(cd); + return configSetName != null + ? new ZkSolrResourceLoader( + cd.getInstanceDir(), configSetName, parentLoader.getClassLoader(), zkController) + : createCoreResourceLoader(cd); } @Override From a32a2acb6a1524a061e40f3c2de53a4e6049e0ee Mon Sep 17 00:00:00 2001 From: patsonluk Date: Mon, 22 Apr 2024 15:44:58 -0700 Subject: [PATCH 16/20] Fixed issue with incorrect collection name in context Ensure only one synthetic core created per config set --- .../solr/servlet/CoordinatorHttpSolrCall.java | 84 ++++++++++++------- 1 file changed, 52 insertions(+), 32 deletions(-) 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 6079b8b009f..c8c2d4b61c8 100644 --- a/solr/core/src/java/org/apache/solr/servlet/CoordinatorHttpSolrCall.java +++ b/solr/core/src/java/org/apache/solr/servlet/CoordinatorHttpSolrCall.java @@ -34,6 +34,7 @@ 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.SolrQueryRequest; import org.slf4j.Logger; @@ -70,8 +71,10 @@ public static SolrCore getCore( Factory factory, HttpSolrCall solrCall, String collectionName, boolean isPreferLeader) { String syntheticCoreName = factory.collectionVsCoreNameMapping.get(collectionName); if (syntheticCoreName != null) { + setMDCLoggingContext(collectionName); return solrCall.cores.getCore(syntheticCoreName); } else { + //first time loading this collection ZkStateReader zkStateReader = solrCall.cores.getZkController().getZkStateReader(); ClusterState clusterState = zkStateReader.getClusterState(); DocCollection coll = clusterState.getCollectionOrNull(collectionName, true); @@ -82,42 +85,46 @@ public static SolrCore getCore( collectionName); return null; } - synchronized (CoordinatorHttpSolrCall.class) { - String confName = coll.getConfigName(); - String syntheticCollectionName = getSyntheticCollectionName(confName); + String confName = coll.getConfigName(); + String syntheticCollectionName = getSyntheticCollectionName(confName); + syntheticCoreName = syntheticCollectionName + "_core"; + + SolrCore syntheticCore; + synchronized (CoordinatorHttpSolrCall.class) { CoreContainer coreContainer = solrCall.cores; - SyntheticSolrCore syntheticCore = - SyntheticSolrCore.createAndRegisterCore( - coreContainer, syntheticCollectionName, coll.getConfigName()); - - // after this point the sync core should be available in the container. Double check - if (coreContainer.getCore(syntheticCore.getName()) != null) { - factory.collectionVsCoreNameMapping.put(collectionName, syntheticCore.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: {}", syntheticCore.getName()); - } - return syntheticCore; + 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, syntheticCollectionName, coll.getConfigName()); } + + 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; + } + }); } - return null; + setMDCLoggingContext(collectionName); + if (log.isDebugEnabled()) { + log.debug("coordinator node, returns synthetic core: {}", syntheticCore.getName()); + } + return syntheticCore; } } @@ -158,6 +165,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<>(); From 02a560fc87612cfeef567117e985f90d3ad90388 Mon Sep 17 00:00:00 2001 From: patsonluk Date: Mon, 22 Apr 2024 15:45:25 -0700 Subject: [PATCH 17/20] ./gradlew tidy --- .../org/apache/solr/servlet/CoordinatorHttpSolrCall.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) 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 c8c2d4b61c8..1f98fae7e44 100644 --- a/solr/core/src/java/org/apache/solr/servlet/CoordinatorHttpSolrCall.java +++ b/solr/core/src/java/org/apache/solr/servlet/CoordinatorHttpSolrCall.java @@ -74,7 +74,7 @@ public static SolrCore getCore( setMDCLoggingContext(collectionName); return solrCall.cores.getCore(syntheticCoreName); } else { - //first time loading this collection + // first time loading this collection ZkStateReader zkStateReader = solrCall.cores.getZkController().getZkStateReader(); ClusterState clusterState = zkStateReader.getClusterState(); DocCollection coll = clusterState.getCollectionOrNull(collectionName, true); @@ -95,11 +95,11 @@ public static SolrCore getCore( CoreContainer coreContainer = solrCall.cores; syntheticCore = coreContainer.getCore(syntheticCoreName); if (syntheticCore == null) { - //first time loading this config set + // first time loading this config set log.info("Loading synthetic core for config set {}", confName); syntheticCore = - SyntheticSolrCore.createAndRegisterCore( - coreContainer, syntheticCollectionName, coll.getConfigName()); + SyntheticSolrCore.createAndRegisterCore( + coreContainer, syntheticCollectionName, coll.getConfigName()); } factory.collectionVsCoreNameMapping.put(collectionName, syntheticCore.getName()); From ef0ed243a2334928f6f40772f16b8df008992b1c Mon Sep 17 00:00:00 2001 From: patsonluk Date: Mon, 22 Apr 2024 18:02:03 -0700 Subject: [PATCH 18/20] Correctly use _core as synthetic core name for clarity Should open the registered core on first synthetic core creation since it is a getCore op --- .../org/apache/solr/core/SyntheticSolrCore.java | 13 +++++++++---- .../solr/servlet/CoordinatorHttpSolrCall.java | 4 +++- 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/solr/core/src/java/org/apache/solr/core/SyntheticSolrCore.java b/solr/core/src/java/org/apache/solr/core/SyntheticSolrCore.java index d77cc013c56..a6fe47d9c5d 100644 --- a/solr/core/src/java/org/apache/solr/core/SyntheticSolrCore.java +++ b/solr/core/src/java/org/apache/solr/core/SyntheticSolrCore.java @@ -22,15 +22,15 @@ public SyntheticSolrCore(CoreContainer coreContainer, CoreDescriptor cd, ConfigS } public static SyntheticSolrCore createAndRegisterCore( - CoreContainer coreContainer, String syntheticCollectionName, String configSetName) { + CoreContainer coreContainer, String syntheticCoreName, String configSetName) { Map coreProps = new HashMap<>(); coreProps.put(CoreAdminParams.CORE_NODE_NAME, coreContainer.getHostName()); - coreProps.put(CoreAdminParams.COLLECTION, syntheticCollectionName); + coreProps.put(CoreAdminParams.COLLECTION, syntheticCoreName); CoreDescriptor syntheticCoreDescriptor = new CoreDescriptor( - syntheticCollectionName, - Paths.get(coreContainer.getSolrHome() + "/" + syntheticCollectionName), + syntheticCoreName, + Paths.get(coreContainer.getSolrHome() + "/" + syntheticCoreName), coreProps, coreContainer.getContainerProperties(), coreContainer.getZkController()); @@ -58,4 +58,9 @@ protected RestManager initRestManager() throws SolrException { // We do not expect RestManager ops on Coordinator Nodes return new RestManager(); } + + @Override + public void close() { + super.close(); + } } 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 1f98fae7e44..896441ac93f 100644 --- a/solr/core/src/java/org/apache/solr/servlet/CoordinatorHttpSolrCall.java +++ b/solr/core/src/java/org/apache/solr/servlet/CoordinatorHttpSolrCall.java @@ -99,7 +99,9 @@ public static SolrCore getCore( log.info("Loading synthetic core for config set {}", confName); syntheticCore = SyntheticSolrCore.createAndRegisterCore( - coreContainer, syntheticCollectionName, coll.getConfigName()); + coreContainer, syntheticCoreName, coll.getConfigName()); + // getting the core should open it + syntheticCore.open(); } factory.collectionVsCoreNameMapping.put(collectionName, syntheticCore.getName()); From 46cfa39f607b1a73347d2b6487dc2540a8a7122f Mon Sep 17 00:00:00 2001 From: patsonluk Date: Mon, 22 Apr 2024 18:03:31 -0700 Subject: [PATCH 19/20] ./gradlew tidy --- solr/core/src/java/org/apache/solr/core/SyntheticSolrCore.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/solr/core/src/java/org/apache/solr/core/SyntheticSolrCore.java b/solr/core/src/java/org/apache/solr/core/SyntheticSolrCore.java index a6fe47d9c5d..b93ddb45455 100644 --- a/solr/core/src/java/org/apache/solr/core/SyntheticSolrCore.java +++ b/solr/core/src/java/org/apache/solr/core/SyntheticSolrCore.java @@ -29,7 +29,7 @@ public static SyntheticSolrCore createAndRegisterCore( CoreDescriptor syntheticCoreDescriptor = new CoreDescriptor( - syntheticCoreName, + syntheticCoreName, Paths.get(coreContainer.getSolrHome() + "/" + syntheticCoreName), coreProps, coreContainer.getContainerProperties(), From 8052d83fc2ec5018c04d9e55f46d7da711176aae Mon Sep 17 00:00:00 2001 From: patsonluk Date: Mon, 22 Apr 2024 19:17:22 -0700 Subject: [PATCH 20/20] Fixed issue with incorrect collection name in context --- .../java/org/apache/solr/servlet/CoordinatorHttpSolrCall.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) 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 896441ac93f..0ec74206c68 100644 --- a/solr/core/src/java/org/apache/solr/servlet/CoordinatorHttpSolrCall.java +++ b/solr/core/src/java/org/apache/solr/servlet/CoordinatorHttpSolrCall.java @@ -71,8 +71,9 @@ public static SolrCore getCore( Factory factory, HttpSolrCall solrCall, String collectionName, boolean isPreferLeader) { String syntheticCoreName = factory.collectionVsCoreNameMapping.get(collectionName); if (syntheticCoreName != null) { + SolrCore syntheticCore = solrCall.cores.getCore(syntheticCoreName); setMDCLoggingContext(collectionName); - return solrCall.cores.getCore(syntheticCoreName); + return syntheticCore; } else { // first time loading this collection ZkStateReader zkStateReader = solrCall.cores.getZkController().getZkStateReader();