This is an automated email from the ASF dual-hosted git repository.
psalagnac pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/solr.git
The following commit(s) were added to refs/heads/main by this push:
new f489a30a31b SOLR-18548: Fix overseer collection lock leak when marking
an async task as running fails (#5167)
f489a30a31b is described below
commit f489a30a31b1b97c7c8aa7a2c05c15d80114dc70
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)
---
.../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;
+ }
+ };
+ }
}
