From c8310ea09d7572bfa7acf55750da536b56d056e2 Mon Sep 17 00:00:00 2001 From: Erick Erickson Date: Sat, 29 Jun 2013 18:40:49 +0000 Subject: [PATCH] SOLR-4974, tighten up shutdown logic. Inspired by SOLR-4960 git-svn-id: https://svn.apache.org/repos/asf/lucene/dev/trunk@1497999 13f79535-47bb-0310-9956-ffa450edef68 --- solr/CHANGES.txt | 3 ++ .../java/org/apache/solr/core/SolrCores.java | 52 ++++--------------- .../collection1/conf/solrconfig-minimal.xml | 2 +- .../solr/core/OpenCloseCoreStressTest.java | 7 ++- 4 files changed, 20 insertions(+), 44 deletions(-) diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt index 6521b54d508..eccac32417b 100644 --- a/solr/CHANGES.txt +++ b/solr/CHANGES.txt @@ -221,6 +221,9 @@ Bug Fixes * SOLR-4926: Fixed rare replication bug that normally only manifested when using compound file format. (yonik, Mark Miller) + +* SOLR-4974: Outgrowth of SOLR-4960 that includes transient cores and pending cores + (Erick Erickson) Optimizations ---------------------- diff --git a/solr/core/src/java/org/apache/solr/core/SolrCores.java b/solr/core/src/java/org/apache/solr/core/SolrCores.java index fd233376d27..6552fb9ae6d 100644 --- a/solr/core/src/java/org/apache/solr/core/SolrCores.java +++ b/solr/core/src/java/org/apache/solr/core/SolrCores.java @@ -101,25 +101,25 @@ class SolrCores { // We are shutting down. You can't hold the lock on the various lists of cores while they shut down, so we need to // make a temporary copy of the names and shut them down outside the lock. protected void close() { - Collection coreList; - List transientNames; - List pendingToClose; + Collection coreList = new ArrayList(); // It might be possible for one of the cores to move from one list to another while we're closing them. So // loop through the lists until they're all empty. In particular, the core could have moved from the transient // list to the pendingCloses list. - while (true) { + do { + coreList.clear(); synchronized (modifyLock) { // make a copy of the cores then clear the map so the core isn't handed out to a request again - coreList = new ArrayList(cores.values()); + coreList.addAll(cores.values()); cores.clear(); - transientNames = new ArrayList(transientCores.keySet()); - pendingToClose = new ArrayList(pendingCloses); - } + coreList.addAll(transientCores.values()); + transientCores.clear(); - if (coreList.size() == 0 && transientNames.size() == 0 && pendingToClose.size() == 0) break; + coreList.addAll(pendingCloses); + pendingCloses.clear(); + } for (SolrCore core : coreList) { try { @@ -128,37 +128,7 @@ class SolrCores { SolrException.log(CoreContainer.log, "Error shutting down core", t); } } - - for (String coreName : transientNames) { - SolrCore core = transientCores.get(coreName); - if (core == null) { - CoreContainer.log.info("Core " + coreName + " moved from transient core container list before closing."); - } else { - try { - core.close(); - } catch (Throwable t) { - SolrException.log(CoreContainer.log, "Error shutting down core", t); - } finally { - synchronized (modifyLock) { - transientCores.remove(coreName); - } - } - } - } - - // We might have some cores that we were _thinking_ about shutting down, so take care of those too. - for (SolrCore core : pendingToClose) { - try { - core.close(); - } catch (Throwable t) { - SolrException.log(CoreContainer.log, "Error shutting down core", t); - } finally { - synchronized (modifyLock) { - pendingCloses.remove(core); - } - } - } - } + } while (coreList.size() > 0); } //WARNING! This should be the _only_ place you put anything into the list of transient cores! @@ -304,7 +274,7 @@ class SolrCores { } /* If you don't increment the reference count, someone could close the core before you use it. */ - protected SolrCore getCoreFromAnyList(String name, boolean incRefCount) { + protected SolrCore getCoreFromAnyList(String name, boolean incRefCount) { synchronized (modifyLock) { SolrCore core = cores.get(name); diff --git a/solr/core/src/test-files/solr/collection1/conf/solrconfig-minimal.xml b/solr/core/src/test-files/solr/collection1/conf/solrconfig-minimal.xml index 6bfd967c73d..78a4eb711d3 100644 --- a/solr/core/src/test-files/solr/collection1/conf/solrconfig-minimal.xml +++ b/solr/core/src/test-files/solr/collection1/conf/solrconfig-minimal.xml @@ -28,7 +28,7 @@ - + diff --git a/solr/core/src/test/org/apache/solr/core/OpenCloseCoreStressTest.java b/solr/core/src/test/org/apache/solr/core/OpenCloseCoreStressTest.java index 4ff58d3a984..3db0f61a08e 100644 --- a/solr/core/src/test/org/apache/solr/core/OpenCloseCoreStressTest.java +++ b/solr/core/src/test/org/apache/solr/core/OpenCloseCoreStressTest.java @@ -207,9 +207,10 @@ public class OpenCloseCoreStressTest extends SolrTestCaseJ4 { } } while (secondsRemaining > 0); - assertTrue("We didn't index any documents, somethings really messsed up", cumulativeDocs > 0); + assertTrue("We didn't index any documents, somethings really messed up", cumulativeDocs > 0); } catch (Exception e) { e.printStackTrace(); + fail("Caught unexpected exception"); } } @@ -241,6 +242,8 @@ public class OpenCloseCoreStressTest extends SolrTestCaseJ4 { FileUtils.copyFile(new File(testConf, "schema-tiny.xml"), new File(conf, "schema-tiny.xml")); FileUtils.copyFile(new File(testConf, "solrconfig-minimal.xml"), new File(conf, "solrconfig-minimal.xml")); + FileUtils.copyFile(new File(testConf, "solrconfig.snippet.randomindexconfig.xml"), + new File(conf, "solrconfig.snippet.randomindexconfig.xml")); if (!oldStyle) { FileUtils.copyFile(new File(testSrcRoot, "conf/core.properties"), new File(coreDir, "core.properties")); @@ -479,7 +482,7 @@ class Queries { try { QueryResponse response = server.query(params); numFound = response.getResults().getNumFound(); - } catch (SolrServerException e) { + } catch (Exception e) { e.printStackTrace(); } return numFound;