From 959ab57a350abceb51388074f4dc23cd467d4f16 Mon Sep 17 00:00:00 2001 From: patsonluk Date: Mon, 26 Feb 2024 11:19:13 -0800 Subject: [PATCH] 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 3af89a4e882..af12039fc53 100644 --- a/solr/core/src/java/org/apache/solr/core/CoreContainer.java +++ b/solr/core/src/java/org/apache/solr/core/CoreContainer.java @@ -1429,8 +1429,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 8f127a8ed8c..48d6dc4610b 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; @@ -206,6 +205,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; @@ -791,7 +791,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); @@ -1053,9 +1054,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( @@ -1066,7 +1071,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); @@ -1176,8 +1182,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); @@ -1204,7 +1212,7 @@ private SolrCore( // searcher! seedVersionBuckets(); - if (!isSynthetic()) { + if (!isSynthetic) { bufferUpdatesIfConstructing(coreDescriptor); } @@ -3099,13 +3107,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()))