From 8367e159e4a287a34adf6552a5aecfe3b8073d8e Mon Sep 17 00:00:00 2001 From: Erick Erickson Date: Wed, 22 Feb 2017 17:46:36 -0800 Subject: [PATCH] SOLR-10021: Cannot reload a core if it fails initialization. --- solr/CHANGES.txt | 2 + .../org/apache/solr/core/CoreContainer.java | 44 ++++++++++--------- .../handler/admin/CoreAdminOperation.java | 6 +-- .../handler/admin/CoreAdminHandlerTest.java | 5 ++- .../client/solrj/request/TestCoreAdmin.java | 39 ++++++++++++++++ 5 files changed, 69 insertions(+), 27 deletions(-) diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt index ed30d53781d..fc5bfe10956 100644 --- a/solr/CHANGES.txt +++ b/solr/CHANGES.txt @@ -244,6 +244,8 @@ Other Changes * SOLR-9848: Lower solr.cloud.wait-for-updates-with-stale-state-pause back down from 7 seconds. (Mark Miller) +* SOLR-10020: Cannot reload a core if it fails initialization. (Mike Drob via Erick Erickson) + ================== 6.4.2 ================== Consult the LUCENE_CHANGES.txt file for additional, low level, changes in this release. 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 f7a8f33a6ca..e3977d7796b 100644 --- a/solr/core/src/java/org/apache/solr/core/CoreContainer.java +++ b/solr/core/src/java/org/apache/solr/core/CoreContainer.java @@ -1116,28 +1116,32 @@ public class CoreContainer { * @param name the name of the SolrCore to reload */ public void reload(String name) { - SolrCore core = solrCores.getCoreFromAnyList(name, false); - if (core == null) - throw new SolrException( SolrException.ErrorCode.BAD_REQUEST, "No such core: " + name ); - - CoreDescriptor cd = core.getCoreDescriptor(); - try { - solrCores.waitAddPendingCoreOps(name); - ConfigSet coreConfig = coreConfigService.getConfig(cd); - log.info("Reloading SolrCore '{}' using configuration from {}", cd.getName(), coreConfig.getName()); - SolrCore newCore = core.reload(coreConfig); - registerCore(name, newCore, false, false); - } catch (SolrCoreState.CoreIsClosedException e) { - throw e; - } catch (Exception e) { - coreInitFailures.put(cd.getName(), new CoreLoadFailure(cd, e)); - throw new SolrException(ErrorCode.SERVER_ERROR, "Unable to reload core [" + cd.getName() + "]", e); + if (core != null) { + CoreDescriptor cd = core.getCoreDescriptor(); + try { + solrCores.waitAddPendingCoreOps(cd.getName()); + ConfigSet coreConfig = coreConfigService.getConfig(cd); + log.info("Reloading SolrCore '{}' using configuration from {}", cd.getName(), coreConfig.getName()); + SolrCore newCore = core.reload(coreConfig); + registerCore(cd.getName(), newCore, false, false); + } catch (SolrCoreState.CoreIsClosedException e) { + throw e; + } catch (Exception e) { + coreInitFailures.put(cd.getName(), new CoreLoadFailure(cd, e)); + throw new SolrException(ErrorCode.SERVER_ERROR, "Unable to reload core [" + cd.getName() + "]", e); + } + finally { + solrCores.removeFromPendingOps(cd.getName()); + } + } else { + CoreLoadFailure clf = coreInitFailures.get(name); + if (clf != null) { + create(clf.cd, true, false); + } else { + throw new SolrException(SolrException.ErrorCode.BAD_REQUEST, "No such core: " + name ); + } } - finally { - solrCores.removeFromPendingOps(name); - } - } /** diff --git a/solr/core/src/java/org/apache/solr/handler/admin/CoreAdminOperation.java b/solr/core/src/java/org/apache/solr/handler/admin/CoreAdminOperation.java index a5782dbd4d3..e7124073e78 100644 --- a/solr/core/src/java/org/apache/solr/handler/admin/CoreAdminOperation.java +++ b/solr/core/src/java/org/apache/solr/handler/admin/CoreAdminOperation.java @@ -102,11 +102,7 @@ enum CoreAdminOperation implements CoreAdminOp { }), RELOAD_OP(RELOAD, it -> { SolrParams params = it.req.getParams(); - String cname = params.get(CoreAdminParams.CORE); - - if (cname == null || !it.handler.coreContainer.getCoreNames().contains(cname)) { - throw new SolrException(ErrorCode.BAD_REQUEST, "Core with core name [" + cname + "] does not exist."); - } + String cname = params.required().get(CoreAdminParams.CORE); try { it.handler.coreContainer.reload(cname); diff --git a/solr/core/src/test/org/apache/solr/handler/admin/CoreAdminHandlerTest.java b/solr/core/src/test/org/apache/solr/handler/admin/CoreAdminHandlerTest.java index 1a596ab3027..a81cf13389d 100644 --- a/solr/core/src/test/org/apache/solr/handler/admin/CoreAdminHandlerTest.java +++ b/solr/core/src/test/org/apache/solr/handler/admin/CoreAdminHandlerTest.java @@ -349,7 +349,8 @@ public class CoreAdminHandlerTest extends SolrTestCaseJ4 { , resp); fail("Was able to successfully reload non-existent-core"); } catch (Exception e) { - assertEquals("Expected error message for non-existent core.", "Core with core name [non-existent-core] does not exist.", e.getMessage()); + String e1 = e.getCause().getMessage(); + assertEquals("Expected error message for non-existent core.", "No such core: non-existent-core", e.getCause().getMessage()); } // test null core @@ -364,7 +365,7 @@ public class CoreAdminHandlerTest extends SolrTestCaseJ4 { if (!(e instanceof SolrException)) { fail("Expected SolrException but got " + e); } - assertEquals("Expected error message for non-existent core.", "Core with core name [null] does not exist.", e.getMessage()); + assertEquals("Expected error message for non-existent core.", "Missing required parameter: core", e.getMessage()); } } diff --git a/solr/solrj/src/test/org/apache/solr/client/solrj/request/TestCoreAdmin.java b/solr/solrj/src/test/org/apache/solr/client/solrj/request/TestCoreAdmin.java index c8c67ec4d18..b2174cd369b 100644 --- a/solr/solrj/src/test/org/apache/solr/client/solrj/request/TestCoreAdmin.java +++ b/solr/solrj/src/test/org/apache/solr/client/solrj/request/TestCoreAdmin.java @@ -19,6 +19,9 @@ package org.apache.solr.client.solrj.request; import java.io.File; import java.io.IOException; import java.lang.invoke.MethodHandles; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.Paths; import java.util.Collection; import com.carrotsearch.randomizedtesting.annotations.ThreadLeakFilters; @@ -41,6 +44,7 @@ import org.apache.solr.common.SolrException; import org.apache.solr.common.SolrInputDocument; import org.apache.solr.common.params.ModifiableSolrParams; import org.apache.solr.common.util.NamedList; +import org.apache.solr.core.CoreContainer; import org.apache.solr.core.SolrCore; import org.apache.solr.metrics.SolrCoreMetricManager; import org.apache.solr.metrics.SolrMetricManager; @@ -292,6 +296,41 @@ public class TestCoreAdmin extends AbstractEmbeddedSolrServerTestCase { expectThrows(SolrException.class, () -> recoverRequestCmd.process(getSolrAdmin())); } + @Test + public void testReloadCoreAfterFailure() throws Exception { + cores.shutdown(); + useFactory(null); // use FS factory + + try { + cores = CoreContainer.createAndLoad(SOLR_HOME, getSolrXml()); + + String ddir = CoreAdminRequest.getCoreStatus("core0", getSolrCore0()).getDataDirectory(); + Path data = Paths.get(ddir, "index"); + assumeTrue("test can't handle relative data directory paths (yet?)", data.isAbsolute()); + + getSolrCore0().add(new SolrInputDocument("id", "core0-1")); + getSolrCore0().commit(); + + cores.shutdown(); + + // destroy the index + Files.move(data.resolve("_0.si"), data.resolve("backup")); + cores = CoreContainer.createAndLoad(SOLR_HOME, getSolrXml()); + + // Need to run a query to confirm that the core couldn't load + expectThrows(SolrException.class, () -> getSolrCore0().query(new SolrQuery("*:*"))); + + // We didn't fix anything, so should still throw + expectThrows(SolrException.class, () -> CoreAdminRequest.reloadCore("core0", getSolrCore0())); + + Files.move(data.resolve("backup"), data.resolve("_0.si")); + CoreAdminRequest.reloadCore("core0", getSolrCore0()); + assertEquals(1, getSolrCore0().query(new SolrQuery("*:*")).getResults().getNumFound()); + } finally { + resetFactory(); + } + } + @BeforeClass public static void before() { // wtf?