This is an automated email from the ASF dual-hosted git repository.
cpoerschke pushed a commit to branch branch_9x
in repository https://gitbox.apache.org/repos/asf/solr.git
The following commit(s) were added to refs/heads/branch_9x by this push:
new b57763e703d SOLR-17269, SOLR-17386: Fixed SyntheticSolrCore reload
issue (#2607)
b57763e703d is described below
commit b57763e703d409c50a2338b51f6fac24f73cce56
Author: patsonluk <[email protected]>
AuthorDate: Fri Aug 9 01:41:51 2024 -0700
SOLR-17269, SOLR-17386: Fixed SyntheticSolrCore reload issue (#2607)
(cherry picked from commit 83e5778af58c5f037da0ce4fdaaf7bef019e9868)
---
solr/CHANGES.txt | 2 +-
.../java/org/apache/solr/core/CoreContainer.java | 5 +-
.../src/java/org/apache/solr/core/SolrCore.java | 41 +++++++-------
.../org/apache/solr/core/SyntheticSolrCore.java | 30 ++++++++++-
.../apache/solr/search/TestCoordinatorRole.java | 63 ++++++++++++++++++++++
5 files changed, 119 insertions(+), 22 deletions(-)
diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt
index 273747359ad..90cffaf2c11 100644
--- a/solr/CHANGES.txt
+++ b/solr/CHANGES.txt
@@ -113,7 +113,7 @@ 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)
+* SOLR-17269, SOLR-17386: Prevent the "Coordinator node" feature from
registering synthetic cores in ZooKeeper (ellaeln, Patson Luk, David Smiley,
Christine Poerschke)
* SOLR-17330: When not set, loadOnStartup defaults to true, which is the
default choice for a core. (Pierre Salagnac via Eric Pugh)
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 a8fb746c910..acf54b24afe 100644
--- a/solr/core/src/java/org/apache/solr/core/CoreContainer.java
+++ b/solr/core/src/java/org/apache/solr/core/CoreContainer.java
@@ -2064,9 +2064,10 @@ public class CoreContainer {
DocCollection docCollection = null;
if (getZkController() != null) {
- docCollection =
getZkController().getClusterState().getCollection(cd.getCollectionName());
+ docCollection =
+
getZkController().getClusterState().getCollectionOrNull(cd.getCollectionName());
// turn off indexing now, before the new core is registered
- if (docCollection.getBool(ZkStateReader.READ_ONLY, false)) {
+ if (docCollection != null &&
docCollection.getBool(ZkStateReader.READ_ONLY, false)) {
newCore.readOnly = true;
}
}
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 77794837cec..f3eb3899fb9 100644
--- a/solr/core/src/java/org/apache/solr/core/SolrCore.java
+++ b/solr/core/src/java/org/apache/solr/core/SolrCore.java
@@ -767,29 +767,16 @@ public class SolrCore implements SolrInfoBean, Closeable {
// only one reload at a time
synchronized (getUpdateHandler().getSolrCoreState().getReloadLock()) {
solrCoreState.increfSolrCoreState();
- final SolrCore currentCore;
- if (!getNewIndexDir().equals(getIndexDir())) {
- // the directory is changing, don't pass on state
- currentCore = null;
- } else {
- currentCore = this;
- }
+
+ // if directory is changing, then don't pass on state
+ boolean cloneCoreState = getNewIndexDir().equals(getIndexDir());
boolean success = false;
SolrCore core = null;
try {
CoreDescriptor cd = new CoreDescriptor(name, getCoreDescriptor());
cd.loadExtraProperties(); // Reload the extra properties
- core =
- new SolrCore(
- coreContainer,
- cd,
- coreConfig,
- getDataDir(),
- updateHandler,
- solrDelPolicy,
- currentCore,
- true);
+ core = cloneForReloadCore(cd, coreConfig, cloneCoreState);
// we open a new IndexWriter to pick up the latest config
core.getUpdateHandler().getSolrCoreState().newIndexWriter(core, false);
@@ -806,6 +793,24 @@ public class SolrCore implements SolrInfoBean, Closeable {
}
}
+ /**
+ * Clones the current core for core reload, with the provided CoreDescriptor
and ConfigSet.
+ *
+ * @return the cloned core to be used for {@link SolrCore#reload}
+ */
+ protected SolrCore cloneForReloadCore(
+ CoreDescriptor newCoreDescriptor, ConfigSet newCoreConfig, boolean
cloneCoreState) {
+ return new SolrCore(
+ coreContainer,
+ newCoreDescriptor,
+ newCoreConfig,
+ getDataDir(),
+ updateHandler,
+ solrDelPolicy,
+ cloneCoreState ? this : null,
+ true);
+ }
+
private DirectoryFactory initDirectoryFactory() {
return DirectoryFactory.loadDirectoryFactory(
solrConfig, coreContainer, coreMetricManager.getRegistryName());
@@ -1055,7 +1060,7 @@ public class SolrCore implements SolrInfoBean, Closeable {
this(coreContainer, cd, configSet, null, null, null, null, false);
}
- private SolrCore(
+ protected SolrCore(
CoreContainer coreContainer,
CoreDescriptor coreDescriptor,
ConfigSet configSet,
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 e97c5780d54..db30474ffd4 100644
--- a/solr/core/src/java/org/apache/solr/core/SyntheticSolrCore.java
+++ b/solr/core/src/java/org/apache/solr/core/SyntheticSolrCore.java
@@ -22,6 +22,7 @@ import java.util.Map;
import org.apache.solr.common.SolrException;
import org.apache.solr.common.params.CoreAdminParams;
import org.apache.solr.rest.RestManager;
+import org.apache.solr.update.UpdateHandler;
/**
* A synthetic core that is created only in memory and not registered against
Zookeeper.
@@ -34,7 +35,20 @@ import org.apache.solr.rest.RestManager;
*/
public class SyntheticSolrCore extends SolrCore {
public SyntheticSolrCore(CoreContainer coreContainer, CoreDescriptor cd,
ConfigSet configSet) {
- super(coreContainer, cd, configSet);
+ this(coreContainer, cd, configSet, null, null, null, null, false);
+ }
+
+ public SyntheticSolrCore(
+ CoreContainer coreContainer,
+ CoreDescriptor coreDescriptor,
+ ConfigSet configSet,
+ String dataDir,
+ UpdateHandler updateHandler,
+ IndexDeletionPolicyWrapper delPolicy,
+ SolrCore prev,
+ boolean reload) {
+ super(
+ coreContainer, coreDescriptor, configSet, dataDir, updateHandler,
delPolicy, prev, reload);
}
public static SyntheticSolrCore createAndRegisterCore(
@@ -72,4 +86,18 @@ public class SyntheticSolrCore extends SolrCore {
// We do not expect RestManager ops on Coordinator Nodes
return new RestManager();
}
+
+ @Override
+ protected SyntheticSolrCore cloneForReloadCore(
+ CoreDescriptor newCoreDescriptor, ConfigSet newCoreConfig, boolean
cloneCurrentState) {
+ return new SyntheticSolrCore(
+ getCoreContainer(),
+ newCoreDescriptor,
+ newCoreConfig,
+ getDataDir(),
+ getUpdateHandler(),
+ getDeletionPolicy(),
+ cloneCurrentState ? this : null,
+ true);
+ }
}
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 70835d7e884..f75448e8b82 100644
--- a/solr/core/src/test/org/apache/solr/search/TestCoordinatorRole.java
+++ b/solr/core/src/test/org/apache/solr/search/TestCoordinatorRole.java
@@ -863,4 +863,67 @@ public class TestCoordinatorRole extends SolrCloudTestCase
{
cluster.shutdown();
}
}
+
+ public void testCoreReload() throws Exception {
+ final int DATA_NODE_COUNT = 1;
+ MiniSolrCloudCluster cluster =
+ configureCluster(DATA_NODE_COUNT)
+ .addConfig("conf1", configset("cloud-minimal"))
+ .configure();
+
+ List<String> dataNodes =
+ cluster.getJettySolrRunners().stream()
+ .map(JettySolrRunner::getNodeName)
+ .collect(Collectors.toUnmodifiableList());
+
+ try {
+ CollectionAdminRequest.createCollection("c1", "conf1", 1,
1).process(cluster.getSolrClient());
+ cluster.waitForActiveCollection("c1", 1, 1);
+
+ System.setProperty(NodeRoles.NODE_ROLES_PROP, "coordinator:on");
+ JettySolrRunner coordinatorJetty;
+ try {
+ coordinatorJetty = cluster.startJettySolrRunner();
+ } finally {
+ System.clearProperty(NodeRoles.NODE_ROLES_PROP);
+ }
+
+ try (HttpSolrClient coordinatorClient =
+ new
HttpSolrClient.Builder(coordinatorJetty.getBaseUrl().toString()).build()) {
+ HttpResponse response =
+ coordinatorClient
+ .getHttpClient()
+ .execute(
+ new HttpGet(
+ coordinatorJetty.getBaseUrl()
+ + "/c1/select?q:*:*")); // make a call so the
synthetic core would be
+ // created
+ assertEquals(200, response.getStatusLine().getStatusCode());
+ // conf1 has no cache-control
+ assertNull(response.getFirstHeader("cache-control"));
+
+ // now update conf1
+ cluster.uploadConfigSet(configset("cache-control"), "conf1");
+
+ response =
+ coordinatorClient
+ .getHttpClient()
+ .execute(
+ new HttpGet(
+ coordinatorJetty.getBaseUrl()
+ +
"/admin/cores?core=.sys.COORDINATOR-COLL-conf1_core&action=reload"));
+ assertEquals(200, response.getStatusLine().getStatusCode());
+
+ response =
+ coordinatorClient
+ .getHttpClient()
+ .execute(new HttpGet(coordinatorJetty.getBaseUrl() +
"/c1/select?q:*:*"));
+ assertEquals(200, response.getStatusLine().getStatusCode());
+ // now the response should show cache-control
+
assertTrue(response.getFirstHeader("cache-control").getValue().contains("max-age=30"));
+ }
+ } finally {
+ cluster.shutdown();
+ }
+ }
}
