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/08/14 18:12:44 UTC
[lucene-solr] 02/04: @541 Getting the close order just absolutely
right is essential.
This is an automated email from the ASF dual-hosted git repository.
markrmiller pushed a commit to branch reference_impl_dev
in repository https://gitbox.apache.org/repos/asf/lucene-solr.git
commit a7b3623f61ac679d56472c4e7e1e9bfd761b64c4
Author: markrmiller@gmail.com <ma...@gmail.com>
AuthorDate: Fri Aug 14 12:33:52 2020 -0500
@541 Getting the close order just absolutely right is essential.
---
.../src/java/org/apache/solr/core/SolrCore.java | 86 +++++++++++-----------
.../apache/solr/handler/ReplicationHandler.java | 10 ++-
.../org/apache/solr/common/ParWorkExecService.java | 45 +++++------
3 files changed, 75 insertions(+), 66 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 486cd56..4ad1250 100644
--- a/solr/core/src/java/org/apache/solr/core/SolrCore.java
+++ b/solr/core/src/java/org/apache/solr/core/SolrCore.java
@@ -1600,6 +1600,49 @@ public final class SolrCore implements SolrInfoBean, Closeable {
searcherExecutor.shutdown();
}
+ AtomicBoolean coreStateClosed = new AtomicBoolean(false);
+
+ closer.add("SolrCoreState", () -> {
+ boolean closed = false;
+ if (updateHandler != null && updateHandler instanceof IndexWriterCloser && solrCoreState != null) {
+ closed = solrCoreState.decrefSolrCoreState((IndexWriterCloser) updateHandler);
+ } else {
+ closed = solrCoreState.decrefSolrCoreState(null);
+ }
+ coreStateClosed.set(closed);
+ return solrCoreState;
+ });
+
+ closer.add("shutdown", () -> {
+
+ synchronized (searcherLock) {
+ while (onDeckSearchers.get() > 0) {
+ try {
+ searcherLock.wait(1000); // nocommit
+ } catch (InterruptedException e) {
+ ParWork.propegateInterrupt(e);
+ } // nocommit
+ }
+ }
+ return "wait for on deck searchers";
+
+ });
+
+ closer.add("closeSearcher", () -> {
+ closeSearcher();
+ });
+ assert ObjectReleaseTracker.release(searcherExecutor);
+ closer.add("searcherExecutor", searcherExecutor, () -> {
+ infoRegistry.clear();
+ return infoRegistry;
+ }, () -> {
+ Directory snapshotsDir = snapshotMgr.getSnapshotsDir();
+ this.directoryFactory.doneWithDirectory(snapshotsDir);
+
+ this.directoryFactory.release(snapshotsDir);
+ return snapshotsDir;
+ });
+
List<Callable<Object>> closeHookCalls = new ArrayList<>();
if (closeHooks != null) {
@@ -1648,49 +1691,6 @@ public final class SolrCore implements SolrInfoBean, Closeable {
closer.add("SolrCoreInternals", closeCalls);
- AtomicBoolean coreStateClosed = new AtomicBoolean(false);
-
- closer.add("SolrCoreState", () -> {
- boolean closed = false;
- if (updateHandler != null && updateHandler instanceof IndexWriterCloser && solrCoreState != null) {
- closed = solrCoreState.decrefSolrCoreState((IndexWriterCloser) updateHandler);
- } else {
- closed = solrCoreState.decrefSolrCoreState(null);
- }
- coreStateClosed.set(closed);
- return solrCoreState;
- });
-
- closer.add("shutdown", () -> {
-
- synchronized (searcherLock) {
- while (onDeckSearchers.get() > 0) {
- try {
- searcherLock.wait(1000); // nocommit
- } catch (InterruptedException e) {
- ParWork.propegateInterrupt(e);
- } // nocommit
- }
- }
- return "wait for on deck searchers";
-
- });
-
- closer.add("closeSearcher", () -> {
- closeSearcher();
- });
- assert ObjectReleaseTracker.release(searcherExecutor);
- closer.add("searcherExecutor", searcherExecutor, () -> {
- infoRegistry.clear();
- return infoRegistry;
- }, () -> {
- Directory snapshotsDir = snapshotMgr.getSnapshotsDir();
- this.directoryFactory.doneWithDirectory(snapshotsDir);
-
- this.directoryFactory.release(snapshotsDir);
- return snapshotsDir;
- });
-
closer.add("CleanupOldIndexDirs", () -> {
if (coreStateClosed.get()) cleanupOldIndexDirectories(false);
});
diff --git a/solr/core/src/java/org/apache/solr/handler/ReplicationHandler.java b/solr/core/src/java/org/apache/solr/handler/ReplicationHandler.java
index c761a25..9bebcca 100644
--- a/solr/core/src/java/org/apache/solr/handler/ReplicationHandler.java
+++ b/solr/core/src/java/org/apache/solr/handler/ReplicationHandler.java
@@ -1417,11 +1417,17 @@ public class ReplicationHandler extends RequestHandlerBase implements SolrCoreAw
core.addCloseHook(new CloseHook() {
@Override
public void preClose(SolrCore core) {
- ExecutorUtil.shutdownAndAwaitTermination(restoreExecutor);
+ try {
+ restoreFuture.cancel(true);
+ ExecutorUtil.shutdownAndAwaitTermination(restoreExecutor);
+ } catch (NullPointerException e) {
+ // okay
+ }
}
@Override
- public void postClose(SolrCore core) {}
+ public void postClose(SolrCore core) {
+ }
});
}
diff --git a/solr/solrj/src/java/org/apache/solr/common/ParWorkExecService.java b/solr/solrj/src/java/org/apache/solr/common/ParWorkExecService.java
index ad0a55f..f0d999e 100644
--- a/solr/solrj/src/java/org/apache/solr/common/ParWorkExecService.java
+++ b/solr/solrj/src/java/org/apache/solr/common/ParWorkExecService.java
@@ -89,7 +89,7 @@ public class ParWorkExecService extends AbstractExecutorService {
service.execute(new Runnable() {
@Override
public void run() {
- runIt(finalRunnable, false);
+ runIt(finalRunnable, true, false, false);
}
});
}
@@ -202,8 +202,9 @@ public class ParWorkExecService extends AbstractExecutorService {
@Override
public void execute(Runnable runnable) {
+
if (shutdown) {
- runIt(runnable, true);
+ runIt(runnable, false, true, true);
return;
}
running.incrementAndGet();
@@ -212,7 +213,7 @@ public class ParWorkExecService extends AbstractExecutorService {
service.execute(new Runnable() {
@Override
public void run() {
- runIt(runnable, false);
+ runIt(runnable, false, false, false);
}
});
} catch (Exception e) {
@@ -224,28 +225,30 @@ public class ParWorkExecService extends AbstractExecutorService {
return;
}
+ boolean acquired = false;
if (runnable instanceof ParWork.SolrFutureTask) {
} else {
-
-
- if (!available.tryAcquire()) {
- runIt(runnable, true);
+ if (!checkLoad()) {
+ runIt(runnable, false, true, false);
return;
}
- if (!checkLoad()) {
- runIt(runnable, true);
+ if (!available.tryAcquire()) {
+ runIt(runnable, false, true, false);
return;
+ } else {
+ acquired = true;
}
}
Runnable finalRunnable = runnable;
try {
- service.execute(new Runnable() {
+ boolean finalAcquired = acquired;
+ service.execute(new Runnable() {
@Override
public void run() {
- runIt(finalRunnable, false);
+ runIt(finalRunnable, finalAcquired, false, false);
}
});
} catch (Exception e) {
@@ -280,24 +283,24 @@ public class ParWorkExecService extends AbstractExecutorService {
// }
}
- private void runIt(Runnable runnable, boolean callThreadRuns) {
+ private void runIt(Runnable runnable, boolean acquired, boolean callThreadRuns, boolean alreadyShutdown) {
try {
runnable.run();
} finally {
try {
- if (runnable instanceof ParWork.SolrFutureTask) {
-
- } else {
+ if (acquired) {
available.release();
}
} finally {
- try {
- running.decrementAndGet();
- synchronized (awaitTerminate) {
- awaitTerminate.notifyAll();
+ if (!alreadyShutdown) {
+ try {
+ running.decrementAndGet();
+ synchronized (awaitTerminate) {
+ awaitTerminate.notifyAll();
+ }
+ } finally {
+ if (!callThreadRuns) ParWork.closeExecutor();
}
- } finally {
- if (!callThreadRuns) ParWork.closeExecutor();
}
}
}