Allow finer grained locking in SolrCores

- Introduces Locks and Conditions for finer grained access to loaded SolrCores - Removes synchronized Object lock - Removes already deprecated TransientSolrCores subclass and their caches to avoid adjusting locking for that class too Co-authored-by: Dennis Berger <[email protected]> Co-authored-by: Torsten Bøgh Köster <[email protected]> Co-authored-by: Marco Petris <[email protected]>
otto-de · Sep 11, 2024 · 1484f6f · 1484f6f
1 parent cf78257
commit 1484f6f
Show file tree

Hide file tree

Showing 8 changed files with 163 additions and 617 deletions.
diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt
@@ -1346,6 +1346,8 @@ Improvements
 
 Optimizations
 ---------------------
+* SOLR-16497 Allow for finer grained locking of access to SolrCores to reduce lock contention
+  (Dennis Berger, Torsten Bøgh Köster, Marco Petris)
 
 * SOLR-16515: Remove synchronized access to cachedOrdMaps in SlowCompositeReaderWrapper (Dennis Berger, Torsten Bøgh Köster, Marco Petris)
 

diff --git a/solr/core/src/java/org/apache/solr/core/CoreContainer.java b/solr/core/src/java/org/apache/solr/core/CoreContainer.java
@@ -1305,17 +1305,25 @@ public void shutdown() {
 
       // First wake up the closer thread, it'll terminate almost immediately since it checks
       // isShutDown.
-      synchronized (solrCores.getModifyLock()) {
-        solrCores.getModifyLock().notifyAll(); // wake up anyone waiting
+      solrCores.getWriteLock().lock();
+      try {
+        solrCores.getWriteLockCondition().signalAll(); // wake up anyone waiting
+      } finally {
+        solrCores.getWriteLock().unlock();
       }
       if (backgroundCloser
           != null) { // Doesn't seem right, but tests get in here without initializing the core.
         try {
           while (true) {
             backgroundCloser.join(15000);
             if (backgroundCloser.isAlive()) {
-              synchronized (solrCores.getModifyLock()) {
-                solrCores.getModifyLock().notifyAll(); // there is a race we have to protect against
+              solrCores.getWriteLock().lock();
+              try {
+                solrCores
+                    .getWriteLockCondition()
+                    .signalAll(); // there is a race we have to protect against
+              } finally {
+                solrCores.getWriteLock().unlock();
               }
             } else {
               break;
@@ -1351,8 +1359,11 @@ public void shutdown() {
       // It's still possible that one of the pending dynamic load operation is waiting, so wake it
       // up if so. Since all the pending operations queues have been drained, there should be
       // nothing to do.
-      synchronized (solrCores.getModifyLock()) {
-        solrCores.getModifyLock().notifyAll(); // wake up the thread
+      solrCores.getWriteLock().lock();
+      try {
+        solrCores.getWriteLockCondition().signalAll(); // wake up the thread
+      } finally {
+        solrCores.getWriteLock().unlock();
       }
 
       customThreadPool.submit(
@@ -2653,13 +2664,16 @@ class CloserThread extends Thread {
   @Override
   public void run() {
     while (!container.isShutDown()) {
-      synchronized (solrCores.getModifyLock()) { // need this so we can wait and be awoken.
+      solrCores.getWriteLock().lock();
+      try { // need this so we can wait and be awoken.
         try {
-          solrCores.getModifyLock().wait();
+          solrCores.getWriteLockCondition().await();
         } catch (InterruptedException e) {
           // Well, if we've been told to stop, we will. Otherwise, continue on and check to see if
           // there are any cores to close.
         }
+      } finally {
+        solrCores.getWriteLock().unlock();
       }
 
       SolrCore core;