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:42 UTC

[lucene-solr] branch reference_impl_dev updated (dcc1989 -> e7995bf)

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

markrmiller pushed a change to branch reference_impl_dev
in repository https://gitbox.apache.org/repos/asf/lucene-solr.git.


    from dcc1989  @539 More ParExecutorService polish.
     new a0c7b1b  @540 Just a tiny bit of ParExecutorService polish.
     new a7b3623  @541 Getting the close order just absolutely right is essential.
     new b701293  @542 Overseer must close like a boss.
     new e7995bf  @543 Have to use the root exec here.

The 4 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:
 .../src/java/org/apache/solr/cloud/Overseer.java   | 43 +++++++----
 .../apache/solr/cloud/OverseerTaskProcessor.java   |  2 +-
 .../src/java/org/apache/solr/core/SolrCore.java    | 87 +++++++++++-----------
 .../apache/solr/handler/ReplicationHandler.java    | 19 ++++-
 .../handler/component/HttpShardHandlerFactory.java |  3 +-
 .../src/java/org/apache/solr/common/ParWork.java   |  2 +-
 .../org/apache/solr/common/ParWorkExecService.java | 46 ++++++------
 .../org/apache/solr/common/ParWorkExecutor.java    | 11 ++-
 8 files changed, 124 insertions(+), 89 deletions(-)


[lucene-solr] 02/04: @541 Getting the close order just absolutely right is essential.

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_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();
         }
       }
     }


[lucene-solr] 01/04: @540 Just a tiny bit of ParExecutorService polish.

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_dev
in repository https://gitbox.apache.org/repos/asf/lucene-solr.git

commit a0c7b1bf93aa21366ce368348641f86e39a0013b
Author: markrmiller@gmail.com <ma...@gmail.com>
AuthorDate: Fri Aug 14 12:20:09 2020 -0500

    @540 Just a tiny bit of ParExecutorService polish.
---
 solr/solrj/src/java/org/apache/solr/common/ParWorkExecService.java | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

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 8074259..ad0a55f 100644
--- a/solr/solrj/src/java/org/apache/solr/common/ParWorkExecService.java
+++ b/solr/solrj/src/java/org/apache/solr/common/ParWorkExecService.java
@@ -203,7 +203,8 @@ public class ParWorkExecService extends AbstractExecutorService {
   @Override
   public void execute(Runnable runnable) {
     if (shutdown) {
-      throw new RejectedExecutionException();
+      runIt(runnable, true);
+      return;
     }
     running.incrementAndGet();
     if (runnable instanceof ParWork.SolrFutureTask) {


[lucene-solr] 04/04: @543 Have to use the root exec here.

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_dev
in repository https://gitbox.apache.org/repos/asf/lucene-solr.git

commit e7995bfb1449fe47f802a49645ca7855479ebdee
Author: markrmiller@gmail.com <ma...@gmail.com>
AuthorDate: Fri Aug 14 13:12:10 2020 -0500

    @543 Have to use the root exec here.
---
 .../org/apache/solr/handler/component/HttpShardHandlerFactory.java     | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/solr/core/src/java/org/apache/solr/handler/component/HttpShardHandlerFactory.java b/solr/core/src/java/org/apache/solr/handler/component/HttpShardHandlerFactory.java
index 34d702f..5d096c5 100644
--- a/solr/core/src/java/org/apache/solr/handler/component/HttpShardHandlerFactory.java
+++ b/solr/core/src/java/org/apache/solr/handler/component/HttpShardHandlerFactory.java
@@ -471,8 +471,7 @@ public class HttpShardHandlerFactory extends ShardHandlerFactory implements org.
    * Creates a new completion service for use by a single set of distributed requests.
    */
   public CompletionService newCompletionService() {
-    return new SolrExecutorCompletionService<ShardResponse>(
-        (ParWorkExecService) ParWork.getExecutor());
+    return new ExecutorCompletionService(ParWork.getEXEC());
   } // ### expert usage
 
 


[lucene-solr] 03/04: @542 Overseer must close like a boss.

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_dev
in repository https://gitbox.apache.org/repos/asf/lucene-solr.git

commit b7012936a53b00f9da571f627daaac5b30eaf76e
Author: markrmiller@gmail.com <ma...@gmail.com>
AuthorDate: Fri Aug 14 13:10:12 2020 -0500

    @542 Overseer must close like a boss.
---
 .../src/java/org/apache/solr/cloud/Overseer.java   | 43 ++++++++++++++--------
 .../apache/solr/cloud/OverseerTaskProcessor.java   |  2 +-
 .../src/java/org/apache/solr/core/SolrCore.java    |  1 +
 .../apache/solr/handler/ReplicationHandler.java    |  9 ++++-
 .../src/java/org/apache/solr/common/ParWork.java   |  2 +-
 .../org/apache/solr/common/ParWorkExecutor.java    | 11 +++++-
 6 files changed, 47 insertions(+), 21 deletions(-)

diff --git a/solr/core/src/java/org/apache/solr/cloud/Overseer.java b/solr/core/src/java/org/apache/solr/cloud/Overseer.java
index 06b19f6..b001915 100644
--- a/solr/core/src/java/org/apache/solr/cloud/Overseer.java
+++ b/solr/core/src/java/org/apache/solr/cloud/Overseer.java
@@ -554,8 +554,8 @@ public class Overseer implements SolrCloseable {
     @Override
     public void close() throws IOException {
       this.isClosed = true;
+      ((Thread)thread).interrupt();
       thread.close();
-      Thread.currentThread().interrupt();
     }
 
     public Closeable getThread() {
@@ -881,31 +881,42 @@ public class Overseer implements SolrCloseable {
     if (log.isDebugEnabled()) {
       log.debug("doClose() - start");
     }
-    try (ParWork closer = new ParWork(this, true)) {
 
-      closer.collect(() -> {
-        IOUtils.closeQuietly(ccThread);
+    if (ccThread != null) {
         ccThread.interrupt();
-      });
-
-      closer.collect(() -> {
-
-        IOUtils.closeQuietly(updaterThread);
-        updaterThread.interrupt();
-      });
+    }
+    if (updaterThread != null) {
+      updaterThread.interrupt();
+    }
 
-      closer.collect(() -> {
+    IOUtils.closeQuietly(ccThread);
 
-        IOUtils.closeQuietly(triggerThread);
-        triggerThread.interrupt();
-      });
+    IOUtils.closeQuietly(updaterThread);
 
-      closer.addCollect("OverseerInternals");
+    if (ccThread != null) {
+      try {
+        ccThread.join();
+      } catch (InterruptedException e) {
+        // okay
+      }
     }
+    if (updaterThread != null) {
+      try {
+        updaterThread.join();
+      } catch (InterruptedException e) {
+        // okay
+      }
+    }
+    //      closer.collect(() -> {
+    //
+    //        IOUtils.closeQuietly(triggerThread);
+    //        triggerThread.interrupt();
+    //      });
 
     if (log.isDebugEnabled()) {
       log.debug("doClose() - end");
     }
+
     assert ObjectReleaseTracker.release(this);
   }
 
diff --git a/solr/core/src/java/org/apache/solr/cloud/OverseerTaskProcessor.java b/solr/core/src/java/org/apache/solr/cloud/OverseerTaskProcessor.java
index 738b959..a1258bf 100644
--- a/solr/core/src/java/org/apache/solr/cloud/OverseerTaskProcessor.java
+++ b/solr/core/src/java/org/apache/solr/cloud/OverseerTaskProcessor.java
@@ -294,7 +294,7 @@ public class OverseerTaskProcessor implements Runnable, Closeable {
                     .getId() + " message:" + message.toString());
             Runner runner = new Runner(messageHandler, message, operation, head,
                 lock);
-            ParWork.getExecutor().submit(runner, true);
+            ParWork.getEXEC().execute(runner);
           }
 
         } catch (InterruptedException | AlreadyClosedException e) {
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 4ad1250..977b5e0 100644
--- a/solr/core/src/java/org/apache/solr/core/SolrCore.java
+++ b/solr/core/src/java/org/apache/solr/core/SolrCore.java
@@ -1632,6 +1632,7 @@ public final class SolrCore implements SolrInfoBean, Closeable {
         closeSearcher();
       });
       assert ObjectReleaseTracker.release(searcherExecutor);
+      searcherExecutor.shutdownNow();
       closer.add("searcherExecutor", searcherExecutor, () -> {
         infoRegistry.clear();
         return infoRegistry;
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 9bebcca..6298ff3 100644
--- a/solr/core/src/java/org/apache/solr/handler/ReplicationHandler.java
+++ b/solr/core/src/java/org/apache/solr/handler/ReplicationHandler.java
@@ -1778,6 +1778,11 @@ public class ReplicationHandler extends RequestHandlerBase implements SolrCoreAw
     if (restoreFuture != null) {
       restoreFuture.cancel(false);
     }
+    try {
+      executorService.shutdownNow();
+    } catch (NullPointerException e) {
+      // okay
+    }
 
     try (ParWork closer = new ParWork(this, true)) {
       if (pollingIndexFetcher != null) {
@@ -1791,8 +1796,8 @@ public class ReplicationHandler extends RequestHandlerBase implements SolrCoreAw
           currentIndexFetcher.destroy();
         });
       }
-      closer.collect(restoreExecutor);
-      closer.collect(executorService);
+    ///  closer.collect(restoreExecutor);
+     // closer.collect(executorService);
       closer.addCollect("ReplicationHandlerClose");
     }
 
diff --git a/solr/solrj/src/java/org/apache/solr/common/ParWork.java b/solr/solrj/src/java/org/apache/solr/common/ParWork.java
index 25fcf6c..bd18584 100644
--- a/solr/solrj/src/java/org/apache/solr/common/ParWork.java
+++ b/solr/solrj/src/java/org/apache/solr/common/ParWork.java
@@ -528,7 +528,7 @@ public class ParWork implements Closeable {
     AtomicReference<Throwable> exception = new AtomicReference<>();
     try {
       for (WorkUnit workUnit : workUnits) {
-        log.info("Process workunit {} {}", workUnit.label, workUnit.objects);
+        if (log.isDebugEnabled()) log.debug("Process workunit {} {}", workUnit.label, workUnit.objects);
         TimeTracker workUnitTracker = null;
         assert (workUnitTracker = workUnit.tracker.startSubClose(workUnit.label)) != null;
         try {
diff --git a/solr/solrj/src/java/org/apache/solr/common/ParWorkExecutor.java b/solr/solrj/src/java/org/apache/solr/common/ParWorkExecutor.java
index 68fdf33..2ba4519 100644
--- a/solr/solrj/src/java/org/apache/solr/common/ParWorkExecutor.java
+++ b/solr/solrj/src/java/org/apache/solr/common/ParWorkExecutor.java
@@ -67,7 +67,16 @@ public class ParWorkExecutor extends ThreadPoolExecutor {
   }
 
   public void shutdown() {
-   // allowCoreThreadTimeOut(true);
+    // wake up idle threads!
+    ThreadPoolExecutor exec = ParWork.getEXEC();
+    for (int i = 0; i < getPoolSize(); i++) {
+      exec.submit(new Runnable() {
+        @Override
+        public void run() {
+
+        }
+      });
+    }
     super.shutdown();
   }
 }