You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@lucene.apache.org by ma...@apache.org on 2020/07/14 21:57:28 UTC
[lucene-solr] branch reference_impl updated (a745b3c -> a7b9847)
This is an automated email from the ASF dual-hosted git repository.
markrmiller pushed a change to branch reference_impl
in repository https://gitbox.apache.org/repos/asf/lucene-solr.git.
from a745b3c #154 - Change SPI reload default.
new 883c3fa #155 - Fix SolrIndexSearcher closer ordering.
new a7b9847 #156 - Pretty much no tests use MetricsHistoryHandler.
The 2 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
Summary of changes:
.../java/org/apache/solr/core/CoreContainer.java | 8 ++-
.../src/java/org/apache/solr/core/SolrCore.java | 81 +++++++++-------------
.../solr/handler/admin/MetricsHistoryHandler.java | 7 +-
.../MetricsHistoryWithAuthIntegrationTest.java | 1 +
.../solr/metrics/SolrCoreMetricManagerTest.java | 1 +
.../src/java/org/apache/solr/SolrTestCase.java | 2 +
6 files changed, 45 insertions(+), 55 deletions(-)
[lucene-solr] 01/02: #155 - Fix SolrIndexSearcher closer ordering.
Posted by ma...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
markrmiller pushed a commit to branch reference_impl
in repository https://gitbox.apache.org/repos/asf/lucene-solr.git
commit 883c3fae04cb2969b804ebcb02ce7fa250017628
Author: markrmiller@gmail.com <ma...@gmail.com>
AuthorDate: Tue Jul 14 16:52:22 2020 -0500
#155 - Fix SolrIndexSearcher closer ordering.
---
.../src/java/org/apache/solr/core/SolrCore.java | 34 ++++++++++++++--------
1 file changed, 22 insertions(+), 12 deletions(-)
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 d866eaf..80edfd3 100644
--- a/solr/core/src/java/org/apache/solr/core/SolrCore.java
+++ b/solr/core/src/java/org/apache/solr/core/SolrCore.java
@@ -1593,14 +1593,15 @@ public final class SolrCore implements SolrInfoBean, Closeable {
synchronized (searcherLock) {
this.isClosed = true;
searcherExecutor.shutdown();
+ try {
+ coreAsyncTaskExecutor.shutdown();
+ } catch (Throwable e) {
+ ParWork.propegateInterrupt(e);
+ }
}
- try {
- coreAsyncTaskExecutor.shutdown();
- } catch (Throwable e) {
- ParWork.propegateInterrupt(e);
- }
+
List<Callable<Object>> closeHookCalls = new ArrayList<>();
@@ -1670,7 +1671,7 @@ public final class SolrCore implements SolrInfoBean, Closeable {
});
}
- closer.add("CloseUpdateHandler&Searcher", coreAsyncTaskExecutor, () -> {
+ closer.add("coreAsyncTaskExecutor", coreAsyncTaskExecutor, () -> {
// Since we waited for the searcherExecutor to shut down,
// there should be no more searchers warming in the background
// that we need to take care of.
@@ -1678,7 +1679,6 @@ public final class SolrCore implements SolrInfoBean, Closeable {
// For the case that a searcher was registered *before* warming
// then the searchExecutor will throw an exception when getSearcher()
// tries to use it, and the exception handling code should close it.
- closeSearcher();
// nocommit
// synchronized (searcherLock) {
// for (RefCounted<SolrIndexSearcher> searcher : _searchers) {
@@ -1705,7 +1705,14 @@ public final class SolrCore implements SolrInfoBean, Closeable {
});
- closer.add("ClearInfoReg&ReleaseSnapShotsDir", () -> {
+ closer.add("closeSearcher", ()->{
+ closeSearcher();
+ });
+
+ closer.add("ClearInfoReg&ReleaseSnapShotsDir", ()->{
+ closeSearcher();
+ return "searcher";
+ }, () -> {
infoRegistry.clear();
return infoRegistry;
}, () -> {
@@ -1899,13 +1906,13 @@ public final class SolrCore implements SolrInfoBean, Closeable {
new SolrNamedThreadFactory("searcherExecutor"));
private AtomicInteger onDeckSearchers = new AtomicInteger(); // number of searchers preparing
// Lock ordering: one can acquire the openSearcherLock and then the searcherLock, but not vice-versa.
- private Object searcherLock = new Object(); // the sync object for the searcher
- private ReentrantLock openSearcherLock = new ReentrantLock(true); // used to serialize opens/reopens for absolute ordering
+ private final Object searcherLock = new Object(); // the sync object for the searcher
+ private final ReentrantLock openSearcherLock = new ReentrantLock(true); // used to serialize opens/reopens for absolute ordering
private final int maxWarmingSearchers; // max number of on-deck searchers allowed
private final int slowQueryThresholdMillis; // threshold above which a query is considered slow
- private RefCounted<SolrIndexSearcher> realtimeSearcher;
- private Callable<DirectoryReader> newReaderCreator;
+ private volatile RefCounted<SolrIndexSearcher> realtimeSearcher;
+ private volatile Callable<DirectoryReader> newReaderCreator;
// For testing
boolean areAllSearcherReferencesEmpty() {
@@ -2172,6 +2179,9 @@ public final class SolrCore implements SolrInfoBean, Closeable {
// (caches take a little while to instantiate)
final boolean useCaches = !realtime;
final String newName = realtime ? "realtime" : "main";
+ if (isClosed() || (getCoreContainer() != null && getCoreContainer().isShutDown())) { // if we start new searchers after close we won't close them
+ throw new SolrCoreState.CoreIsClosedException();
+ }
tmp = new SolrIndexSearcher(this, newIndexDir, getLatestSchema(), newName,
newReader, true, useCaches, true, directoryFactory);
[lucene-solr] 02/02: #156 - Pretty much no tests use
MetricsHistoryHandler.
Posted by ma...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
markrmiller pushed a commit to branch reference_impl
in repository https://gitbox.apache.org/repos/asf/lucene-solr.git
commit a7b9847a7e98c27d74b13ac0a46d3e36945fbba3
Author: markrmiller@gmail.com <ma...@gmail.com>
AuthorDate: Tue Jul 14 16:57:11 2020 -0500
#156 - Pretty much no tests use MetricsHistoryHandler.
---
.../java/org/apache/solr/core/CoreContainer.java | 8 ++--
.../src/java/org/apache/solr/core/SolrCore.java | 55 ++++++----------------
.../solr/handler/admin/MetricsHistoryHandler.java | 7 +--
.../MetricsHistoryWithAuthIntegrationTest.java | 1 +
.../solr/metrics/SolrCoreMetricManagerTest.java | 1 +
.../src/java/org/apache/solr/SolrTestCase.java | 2 +
6 files changed, 27 insertions(+), 47 deletions(-)
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 c9a5aa1..c9ad160 100644
--- a/solr/core/src/java/org/apache/solr/core/CoreContainer.java
+++ b/solr/core/src/java/org/apache/solr/core/CoreContainer.java
@@ -781,9 +781,11 @@ public class CoreContainer implements Closeable {
metricsHandler.initializeMetrics(solrMetricsContext, METRICS_PATH);
});
- work.collect(() -> {
- createMetricsHistoryHandler();
- });
+ if (!Boolean.getBoolean("solr.disableMetricsHistoryHandler")) {
+ work.collect(() -> {
+ createMetricsHistoryHandler();
+ });
+ }
work.collect(() -> {
autoscalingHistoryHandler = createHandler(AUTOSCALING_HISTORY_PATH, AutoscalingHistoryHandler.class.getName(), AutoscalingHistoryHandler.class);
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 80edfd3..a192ab9 100644
--- a/solr/core/src/java/org/apache/solr/core/SolrCore.java
+++ b/solr/core/src/java/org/apache/solr/core/SolrCore.java
@@ -1549,11 +1549,6 @@ public final class SolrCore implements SolrInfoBean, Closeable {
MDCLoggingContext.setCore(this);
}
- @Override
- public void close() {
- close(false);
- }
-
/**
* Close all resources allocated by the core if it is no longer in use...
* <ul>
@@ -1579,8 +1574,8 @@ public final class SolrCore implements SolrInfoBean, Closeable {
*
* @see #isClosed()
*/
-
- public void close(boolean failedInConstructor) {
+ @Override
+ public void close() {
int count = refCount.decrementAndGet();
if (count > 0) return; // close is called often, and only actually closes if nothing is using it.
if (count < 0) {
@@ -1600,9 +1595,6 @@ public final class SolrCore implements SolrInfoBean, Closeable {
}
}
-
-
-
List<Callable<Object>> closeHookCalls = new ArrayList<>();
if (closeHooks != null) {
@@ -1616,9 +1608,6 @@ public final class SolrCore implements SolrInfoBean, Closeable {
assert ObjectReleaseTracker.release(searcherExecutor);
- // closer.add("PreCloseHooks", closeHookCalls);
-
-
List<Callable<Object>> closeCalls = new ArrayList<Callable<Object>>();
closeCalls.addAll(closeHookCalls);
closeCalls.add(() -> {
@@ -1658,34 +1647,18 @@ public final class SolrCore implements SolrInfoBean, Closeable {
AtomicBoolean coreStateClosed = new AtomicBoolean(false);
- if (!failedInConstructor) {
- closer.add("SolrCoreState", () -> {
- boolean closed = false;
- if (updateHandler != null && updateHandler instanceof IndexWriterCloser) {
- closed = solrCoreState.decrefSolrCoreState((IndexWriterCloser) updateHandler);
- } else {
- closed = solrCoreState.decrefSolrCoreState(null);
- }
- coreStateClosed.set(closed);
- return solrCoreState;
- });
- }
+ closer.add("SolrCoreState", () -> {
+ boolean closed = false;
+ if (updateHandler != null && updateHandler instanceof IndexWriterCloser) {
+ closed = solrCoreState.decrefSolrCoreState((IndexWriterCloser) updateHandler);
+ } else {
+ closed = solrCoreState.decrefSolrCoreState(null);
+ }
+ coreStateClosed.set(closed);
+ return solrCoreState;
+ });
closer.add("coreAsyncTaskExecutor", coreAsyncTaskExecutor, () -> {
- // Since we waited for the searcherExecutor to shut down,
- // there should be no more searchers warming in the background
- // that we need to take care of.
- //
- // For the case that a searcher was registered *before* warming
- // then the searchExecutor will throw an exception when getSearcher()
- // tries to use it, and the exception handling code should close it.
-// nocommit
-// synchronized (searcherLock) {
-// for (RefCounted<SolrIndexSearcher> searcher : _searchers) {
-// searcher.decref();
-// }
-// }
-
return "Searcher";
});
@@ -1705,11 +1678,11 @@ public final class SolrCore implements SolrInfoBean, Closeable {
});
- closer.add("closeSearcher", ()->{
+ closer.add("closeSearcher", () -> {
closeSearcher();
});
- closer.add("ClearInfoReg&ReleaseSnapShotsDir", ()->{
+ closer.add("ClearInfoReg&ReleaseSnapShotsDir", () -> {
closeSearcher();
return "searcher";
}, () -> {
diff --git a/solr/core/src/java/org/apache/solr/handler/admin/MetricsHistoryHandler.java b/solr/core/src/java/org/apache/solr/handler/admin/MetricsHistoryHandler.java
index edda4ab..4378d65 100644
--- a/solr/core/src/java/org/apache/solr/handler/admin/MetricsHistoryHandler.java
+++ b/solr/core/src/java/org/apache/solr/handler/admin/MetricsHistoryHandler.java
@@ -665,11 +665,12 @@ public class MetricsHistoryHandler extends RequestHandlerBase implements Permiss
if (log.isDebugEnabled()) {
log.debug("Closing {}", hashCode());
}
- collectService.shutdownNow();
+
try (ParWork closer = new ParWork(this)) {
- closer.collect(collectService);
closer.collect(factory);
- closer.addCollect("metricsHistoryHandlerClose");
+ closer.addCollect("factory");
+ closer.collect(collectService);
+ closer.addCollect("collectService");
}
knownDbs.clear();
diff --git a/solr/core/src/test/org/apache/solr/cloud/MetricsHistoryWithAuthIntegrationTest.java b/solr/core/src/test/org/apache/solr/cloud/MetricsHistoryWithAuthIntegrationTest.java
index 48699a1..259ca8c 100644
--- a/solr/core/src/test/org/apache/solr/cloud/MetricsHistoryWithAuthIntegrationTest.java
+++ b/solr/core/src/test/org/apache/solr/cloud/MetricsHistoryWithAuthIntegrationTest.java
@@ -61,6 +61,7 @@ public class MetricsHistoryWithAuthIntegrationTest extends SolrCloudTestCase {
@BeforeClass
public static void setupCluster() throws Exception {
+ System.setProperty("solr.disableMetricsHistoryHandler", "false");
System.setProperty("solr.disableJmxReporter", "false");
String solrXml = MiniSolrCloudCluster.DEFAULT_CLOUD_SOLR_XML.replace("<metrics>\n",
"<metrics>\n" + SOLR_XML_HISTORY_CONFIG);
diff --git a/solr/core/src/test/org/apache/solr/metrics/SolrCoreMetricManagerTest.java b/solr/core/src/test/org/apache/solr/metrics/SolrCoreMetricManagerTest.java
index 141537b..9e52d33 100644
--- a/solr/core/src/test/org/apache/solr/metrics/SolrCoreMetricManagerTest.java
+++ b/solr/core/src/test/org/apache/solr/metrics/SolrCoreMetricManagerTest.java
@@ -44,6 +44,7 @@ public class SolrCoreMetricManagerTest extends SolrTestCaseJ4 {
@Before
public void beforeTest() throws Exception {
+ useFactory(null);
initCore("solrconfig-basic.xml", "schema.xml");
coreMetricManager = h.getCore().getCoreMetricManager();
metricManager = h.getCore().getCoreContainer().getMetricManager();
diff --git a/solr/test-framework/src/java/org/apache/solr/SolrTestCase.java b/solr/test-framework/src/java/org/apache/solr/SolrTestCase.java
index 5c7070a..a7fe0f8 100644
--- a/solr/test-framework/src/java/org/apache/solr/SolrTestCase.java
+++ b/solr/test-framework/src/java/org/apache/solr/SolrTestCase.java
@@ -194,6 +194,8 @@ public class SolrTestCase extends LuceneTestCase {
// nocommit - not used again yet
System.setProperty("solr.OverseerStateUpdateDelay", "0");
+ System.setProperty("solr.disableMetricsHistoryHandler", "true");
+
System.setProperty("solr.leaderThrottle", "0");
System.setProperty("solr.recoveryThrottle", "0");