This is an automated email from the ASF dual-hosted git repository. dsmiley pushed a commit to branch branch_10x in repository https://gitbox.apache.org/repos/asf/solr.git
commit a2626d839af4d63f78b067063c409fcef7b0a7fe Author: Boyi Dai <[email protected]> AuthorDate: Sun Nov 1 16:53:16 2026 -0500 SOLR-18548: Fix overseer collection lock leak when marking an async task as running fails (#5167) (cherry picked from commit f489a30a31b1b97c7c8aa7a2c05c15d80114dc70) --- .../unreleased/SOLR-18548-overseer-lock-leak.yml | 12 ++ .../apache/solr/cloud/OverseerTaskProcessor.java | 4 + .../OverseerCollectionConfigSetProcessorTest.java | 138 ++++++++++++++------- 3 files changed, 112 insertions(+), 42 deletions(-) diff --git a/changelog/unreleased/SOLR-18548-overseer-lock-leak.yml b/changelog/unreleased/SOLR-18548-overseer-lock-leak.yml new file mode 100644 index 00000000000..3f66cfa076b --- /dev/null +++ b/changelog/unreleased/SOLR-18548-overseer-lock-leak.yml @@ -0,0 +1,12 @@ +# See https://github.com/apache/solr/blob/main/dev-docs/changelog.adoc + +title: > + The Overseer now releases a task's lock when storing its async ID in ZooKeeper fails, instead of + blocking later tasks for the same collection until the Overseer restarts +type: fixed +authors: + - name: Boyi Dai + nick: SpookyJumpyBeans +links: + - name: SOLR-18548 + url: https://issues.apache.org/jira/browse/SOLR-18548 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 37dff20a2f4..9448118c9d0 100644 --- a/solr/core/src/java/org/apache/solr/cloud/OverseerTaskProcessor.java +++ b/solr/core/src/java/org/apache/solr/cloud/OverseerTaskProcessor.java @@ -382,6 +382,10 @@ public class OverseerTaskProcessor implements SolrInfoBean, Runnable, Closeable "Thread interrupted while trying to pick task {} for execution.", head.getId()); Thread.currentThread().interrupt(); continue; + } catch (KeeperException e) { + // No Runner was created, so nothing else will release the lock. + lock.unlock(); + throw e; } if (log.isDebugEnabled()) { log.debug( diff --git a/solr/core/src/test/org/apache/solr/cloud/OverseerCollectionConfigSetProcessorTest.java b/solr/core/src/test/org/apache/solr/cloud/OverseerCollectionConfigSetProcessorTest.java index 2608a4e8635..64684ebc4ea 100644 --- a/solr/core/src/test/org/apache/solr/cloud/OverseerCollectionConfigSetProcessorTest.java +++ b/solr/core/src/test/org/apache/solr/cloud/OverseerCollectionConfigSetProcessorTest.java @@ -22,10 +22,12 @@ import static org.mockito.Mockito.anyInt; import static org.mockito.Mockito.anyLong; import static org.mockito.Mockito.anyString; import static org.mockito.Mockito.doAnswer; +import static org.mockito.Mockito.doThrow; import static org.mockito.Mockito.eq; import static org.mockito.Mockito.isNull; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.reset; +import static org.mockito.Mockito.timeout; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; @@ -1447,16 +1449,7 @@ public class OverseerCollectionConfigSetProcessorTest extends SolrTestCaseJ4 { String asyncId = "lock-fail-test-async-123"; // Create a real OverseerCollectionMessageHandler so we exercise the real LockTree locking - OverseerCollectionMessageHandler collHandler = - new OverseerCollectionMessageHandler( - zkStateReaderMock, - "1234", - shardHandlerFactoryMock, - ADMIN_PATH, - new Stats(), - overseerMock, - new OverseerNodePrioritizer( - zkStateReaderMock, overseerMock, ADMIN_PATH, shardHandlerFactoryMock)); + OverseerCollectionMessageHandler collHandler = newCollectionMessageHandler(); // Acquire a lock on collA by calling lockTask directly. // This puts a real lock into the LockTree's allLocks map. @@ -1471,38 +1464,8 @@ public class OverseerCollectionConfigSetProcessorTest extends SolrTestCaseJ4 { assertNotNull("Should have acquired lock on collA", collALock); String collALockId = collALock.id(); - // Build a selector that always returns our real handler - OverseerTaskProcessor.OverseerMessageHandlerSelector selector = - new OverseerTaskProcessor.OverseerMessageHandlerSelector() { - @Override - public void close() { - IOUtils.closeQuietly(collHandler); - } - - @Override - public OverseerMessageHandler selectOverseerMessageHandler(ZkNodeProps message) { - return collHandler; - } - }; - - // Create a processor using the real handler - OverseerTaskProcessor processor = - new OverseerTaskProcessor( - zkStateReaderMock, - "1234", - new Stats(), - selector, - mock(OverseerNodePrioritizer.class), - workQueueMock, - runningMapMock, - completedMapMock, - failureMapMock, - solrMetricsContextMock) { - @Override - protected LeaderStatus amILeader() { - return LeaderStatus.YES; - } - }; + // Create a processor that always routes tasks to our real handler + OverseerTaskProcessor processor = newProcessor(collHandler); Thread processorThread = new Thread(processor); processorThread.start(); @@ -1539,4 +1502,95 @@ public class OverseerCollectionConfigSetProcessorTest extends SolrTestCaseJ4 { processorThread.join(MAX_WAIT_MS); } } + + /** + * Verify that when marking an async task as running in ZooKeeper fails with an unexpected + * KeeperException, the collection lock taken for that task is released. Otherwise no later task + * for the collection could acquire it. + */ + @Test + public void testRunningMapFailureReleasesCollectionLock() throws Exception { + commonMocks(2, false); + + String asyncId = "running-map-fail-test-async-123"; + doThrow(new KeeperException.OperationTimeoutException()) + .when(runningMapMock) + .put(eq(asyncId), any()); + + OverseerCollectionMessageHandler collHandler = newCollectionMessageHandler(); + + OverseerTaskProcessor processor = newProcessor(collHandler); + + Thread processorThread = new Thread(processor); + processorThread.start(); + + ZkNodeProps collAMessage = + new ZkNodeProps( + Map.of( + Overseer.QUEUE_OPERATION, + CollectionParams.CollectionAction.MOCK_COLL_TASK.toLower(), + "name", + "collA", + "async", + asyncId)); + try { + queue.add(new QueueEvent("runningMapFailTask", Utils.toJSON(collAMessage), null)); + verify(runningMapMock, timeout(MAX_WAIT_MS).atLeastOnce()).put(eq(asyncId), any()); + } finally { + processor.close(); + processorThread.interrupt(); + processorThread.join(MAX_WAIT_MS); + } + + // A fresh session ignores the processor's busy markers, so this only fails if a lock on collA + // is still held. + OverseerMessageHandler.Lock lock = collHandler.lockTask(collAMessage, Long.MAX_VALUE, null); + assertNotNull("The lock on collA was not released after the running map failure", lock); + lock.unlock(); + } + + /** Creates a real OverseerCollectionMessageHandler, so tests exercise the real LockTree. */ + private OverseerCollectionMessageHandler newCollectionMessageHandler() { + return new OverseerCollectionMessageHandler( + zkStateReaderMock, + "1234", + shardHandlerFactoryMock, + ADMIN_PATH, + new Stats(), + overseerMock, + new OverseerNodePrioritizer( + zkStateReaderMock, overseerMock, ADMIN_PATH, shardHandlerFactoryMock)); + } + + /** Creates a processor that always routes tasks to the given handler. */ + private OverseerTaskProcessor newProcessor(OverseerCollectionMessageHandler collHandler) { + OverseerTaskProcessor.OverseerMessageHandlerSelector selector = + new OverseerTaskProcessor.OverseerMessageHandlerSelector() { + @Override + public void close() { + IOUtils.closeQuietly(collHandler); + } + + @Override + public OverseerMessageHandler selectOverseerMessageHandler(ZkNodeProps message) { + return collHandler; + } + }; + return new OverseerTaskProcessor( + zkStateReaderMock, + "1234", + new Stats(), + selector, + mock(OverseerNodePrioritizer.class), + workQueueMock, + runningMapMock, + completedMapMock, + failureMapMock, + solrMetricsContextMock) { + @Override + protected LeaderStatus amILeader() { + return LeaderStatus.YES; + } + }; + } }
