This is an automated email from the ASF dual-hosted git repository.

cpoerschke pushed a commit to branch branch_9_7
in repository https://gitbox.apache.org/repos/asf/solr.git


The following commit(s) were added to refs/heads/branch_9_7 by this push:
     new 81cdb9d49cd SOLR-17269, SOLR-17386: Fixed SyntheticSolrCore reload 
issue (#2607)
81cdb9d49cd is described below

commit 81cdb9d49cd28d0856b0f4e65e5bb1b40026af97
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)
    (cherry picked from commit b57763e703d409c50a2338b51f6fac24f73cce56)
---
 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 baa1247f13d..963079d2793 100644
--- a/solr/CHANGES.txt
+++ b/solr/CHANGES.txt
@@ -81,7 +81,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 cf664185046..27c6c3f1b00 100644
--- a/solr/core/src/java/org/apache/solr/core/CoreContainer.java
+++ b/solr/core/src/java/org/apache/solr/core/CoreContainer.java
@@ -2063,9 +2063,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();
+    }
+  }
 }

Reply via email to