This is an automated email from the ASF dual-hosted git repository.
dsmiley 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 0d48fb5b1af SOLR-17895: When Overseer is disabled: Remove unused lock
nodes (#5161)
0d48fb5b1af is described below
commit 0d48fb5b1afdc985f1577576d32deb3d649a418e
Author: sarkaramrit1993 <[email protected]>
AuthorDate: Mon Oct 26 06:00:13 2026 -0700
SOLR-17895: When Overseer is disabled: Remove unused lock nodes (#5161)
by using modern Zookeeper "container znodes"
---
.../unreleased/SOLR-17895-lock-znode-leak.yml | 8 +
.../cloud/ZkDistributedCollectionLockFactory.java | 19 +-
.../cloud/ZkDistributedConfigSetLockFactory.java | 9 +-
.../solr/cloud/ZkDistributedLockFactory.java | 66 ++++--
...istributedCollectionConfigSetCommandRunner.java | 30 ++-
.../apache/solr/cloud/ZkDistributedLockTest.java | 248 +++++++++++++++++++++
.../DeleteCollectionLockCleanupTest.java | 79 +++++++
.../pages/major-changes-in-solr-10.adoc | 2 +
8 files changed, 428 insertions(+), 33 deletions(-)
diff --git a/changelog/unreleased/SOLR-17895-lock-znode-leak.yml
b/changelog/unreleased/SOLR-17895-lock-znode-leak.yml
new file mode 100644
index 00000000000..01e047ab4cd
--- /dev/null
+++ b/changelog/unreleased/SOLR-17895-lock-znode-leak.yml
@@ -0,0 +1,8 @@
+# See https://github.com/apache/solr/blob/main/dev-docs/changelog.adoc
+title: With the Overseer disabled, lock nodes in ZooKeeper are now removed
once no longer used.
+type: fixed # added, changed, fixed, deprecated, removed, dependency_update,
security, other
+authors:
+ - name: Amrit Sarkar
+links:
+ - name: SOLR-17895
+ url: https://issues.apache.org/jira/browse/SOLR-17895
diff --git
a/solr/core/src/java/org/apache/solr/cloud/ZkDistributedCollectionLockFactory.java
b/solr/core/src/java/org/apache/solr/cloud/ZkDistributedCollectionLockFactory.java
index 098545b6afc..54a17b919ca 100644
---
a/solr/core/src/java/org/apache/solr/cloud/ZkDistributedCollectionLockFactory.java
+++
b/solr/core/src/java/org/apache/solr/cloud/ZkDistributedCollectionLockFactory.java
@@ -22,11 +22,13 @@ import
org.apache.solr.cloud.api.collections.DistributedCollectionConfigSetComma
import org.apache.solr.common.SolrException;
import org.apache.solr.common.cloud.SolrZkClient;
import org.apache.solr.common.params.CollectionParams;
+import org.apache.zookeeper.KeeperException;
/**
* A distributed lock implementation using Zookeeper "directory" nodes created
within a collection
* znode hierarchy, for use with the distributed Collection API
implementation. The locks are
- * implemented using ephemeral nodes placed below the "directory" nodes.
+ * implemented using ephemeral nodes placed below the "directory" nodes, which
are container nodes
+ * so ZooKeeper removes them once empty.
*
* @see <a
href="https://zookeeper.apache.org/doc/current/recipes.html#sc_recipes_Locks";>Zookeeper
* lock recipe</a>
@@ -62,8 +64,16 @@ public class ZkDistributedCollectionLockFactory extends
ZkDistributedLockFactory
}
/**
- * Returns the Zookeeper path to the lock, creating missing nodes if needed.
Note that the
- * complete lock hierarchy (/locks and below) can be deleted if SolrCloud is
stopped.
+ * Deletes the empty lock dirs left for {@code collName}. Clusters that ran
older versions have
+ * these as persistent nodes, which ZooKeeper never removes on its own.
+ */
+ public void deleteEmptyLockDirs(String collName) throws KeeperException,
InterruptedException {
+ deleteEmptyDirs(getPathPrefix().append(collName).toString());
+ }
+
+ /**
+ * Returns the Zookeeper path to the lock. Note that the complete lock
hierarchy (/locks and
+ * below) can be deleted if SolrCloud is stopped.
*
* <p>The tree of lock directories for a given collection {@code collName}
is as follows:
*
@@ -81,8 +91,7 @@ public class ZkDistributedCollectionLockFactory extends
ZkDistributedLockFactory
* _replicaNameS2R2 <-- EPHEMERAL replica level locks go here
* }</pre>
*
- * This method will create the path where the {@code EPHEMERAL} lock nodes
should go. That path
- * is:
+ * The path where the {@code EPHEMERAL} lock nodes should go is:
*
* <ul>
* <li>For {@link
org.apache.solr.common.params.CollectionParams.LockLevel#COLLECTION} - {@code
diff --git
a/solr/core/src/java/org/apache/solr/cloud/ZkDistributedConfigSetLockFactory.java
b/solr/core/src/java/org/apache/solr/cloud/ZkDistributedConfigSetLockFactory.java
index 3458ea32215..c3da79044e7 100644
---
a/solr/core/src/java/org/apache/solr/cloud/ZkDistributedConfigSetLockFactory.java
+++
b/solr/core/src/java/org/apache/solr/cloud/ZkDistributedConfigSetLockFactory.java
@@ -23,7 +23,8 @@ import org.apache.solr.common.cloud.SolrZkClient;
/**
* A distributed lock implementation using Zookeeper "directory" nodes created
within a collection
* znode hierarchy, for use with the distributed Collection API
implementation. The locks are
- * implemented using ephemeral nodes placed below the "directory" nodes.
+ * implemented using ephemeral nodes placed below the "directory" nodes, which
are container nodes
+ * so ZooKeeper removes them once empty.
*
* @see <a
href="https://zookeeper.apache.org/doc/current/recipes.html#sc_recipes_Locks";>Zookeeper
* lock recipe</a>
@@ -44,8 +45,8 @@ public class ZkDistributedConfigSetLockFactory extends
ZkDistributedLockFactory
}
/**
- * Returns the Zookeeper path to the lock, creating missing nodes if needed.
Note that the
- * complete lock hierarchy (/rootPath and below) can be deleted if SolrCloud
is stopped.
+ * Returns the Zookeeper path to the lock. Note that the complete lock
hierarchy (/rootPath and
+ * below) can be deleted if SolrCloud is stopped.
*
* <p>The tree of lock directories is very flat, given there's no real
structure to what's being
* locked in a config set:
@@ -57,8 +58,6 @@ public class ZkDistributedConfigSetLockFactory extends
ZkDistributedLockFactory
* etc...
* }</pre>
*
- * This method will create the path where the {@code EPHEMERAL} lock nodes
should go.
- *
* <p>The returned path does not contain the separator ({@code "/"}) at the
end.
*/
private String getLockPath(String configSetName) {
diff --git
a/solr/core/src/java/org/apache/solr/cloud/ZkDistributedLockFactory.java
b/solr/core/src/java/org/apache/solr/cloud/ZkDistributedLockFactory.java
index 969e65a6aaa..90d475faac2 100644
--- a/solr/core/src/java/org/apache/solr/cloud/ZkDistributedLockFactory.java
+++ b/solr/core/src/java/org/apache/solr/cloud/ZkDistributedLockFactory.java
@@ -20,12 +20,14 @@ package org.apache.solr.cloud;
import
org.apache.solr.cloud.api.collections.DistributedCollectionConfigSetCommandRunner;
import org.apache.solr.common.SolrException;
import org.apache.solr.common.cloud.SolrZkClient;
-import org.apache.solr.common.util.StrUtils;
import org.apache.zookeeper.CreateMode;
import org.apache.zookeeper.KeeperException;
+import org.apache.zookeeper.data.Stat;
abstract class ZkDistributedLockFactory {
+ static final int MAX_CREATE_ATTEMPTS = 3;
+
private final SolrZkClient zkClient;
private final String rootPath;
@@ -37,16 +39,19 @@ abstract class ZkDistributedLockFactory {
protected DistributedLock doCreateLock(
boolean isWriteLock, String lockPath, String lockIdToMirror) {
try {
- if (StrUtils.isBlank(lockIdToMirror)) {
- // TODO optimize by first attempting to create the ZkDistributedLock
without calling
- // makeLockPath() and only call it if the lock creation fails. This
will be less costly on
- // high contention (and slightly more on low contention)
- makeLockPath(lockPath);
+ for (int attempt = 1; ; attempt++) {
+ try {
+ return isWriteLock
+ ? new ZkDistributedLock.Write(zkClient, lockPath, lockIdToMirror)
+ : new ZkDistributedLock.Read(zkClient, lockPath, lockIdToMirror);
+ } catch (KeeperException.NoNodeException e) {
+ if (attempt == MAX_CREATE_ATTEMPTS) {
+ throw e;
+ }
+ // Lock dirs are containers, so ZK drops them when empty. Recreate
and try again.
+ makeLockPath(lockPath);
+ }
}
-
- return isWriteLock
- ? new ZkDistributedLock.Write(zkClient, lockPath, lockIdToMirror)
- : new ZkDistributedLock.Read(zkClient, lockPath, lockIdToMirror);
} catch (KeeperException e) {
throw new SolrException(SolrException.ErrorCode.SERVER_ERROR, e);
} catch (InterruptedException e) {
@@ -61,13 +66,44 @@ abstract class ZkDistributedLockFactory {
return path;
}
- private void makeLockPath(String lockNodePath) throws KeeperException,
InterruptedException {
+ private void makeLockPath(String lockPath) throws KeeperException,
InterruptedException {
+ if (!zkClient.exists(rootPath)) {
+ zkClient.makePath(rootPath, false);
+ }
+ StringBuilder path = new StringBuilder(rootPath);
+ for (String node :
+ lockPath
+ .substring(rootPath.length() + 1)
+
.split(DistributedCollectionConfigSetCommandRunner.ZK_PATH_SEPARATOR)) {
+
path.append(DistributedCollectionConfigSetCommandRunner.ZK_PATH_SEPARATOR).append(node);
+ try {
+ zkClient.create(path.toString(), null, CreateMode.CONTAINER);
+ } catch (KeeperException.NodeExistsException nee) {
+ // Some other thread (on this or another JVM) beat us to create the
node, that's ok.
+ } catch (KeeperException.NoNodeException nne) {
+ // A parent got removed while we were here. The caller retries.
+ return;
+ }
+ }
+ }
+
+ /**
+ * Deletes the empty lock dirs under {@code path}, deepest first. Lock nodes
and dirs still
+ * holding them are left alone.
+ */
+ protected void deleteEmptyDirs(String path) throws KeeperException,
InterruptedException {
+ Stat stat = zkClient.exists(path, null);
+ if (stat == null || stat.getEphemeralOwner() != 0) {
+ return;
+ }
try {
- if (!zkClient.exists(lockNodePath)) {
- zkClient.makePath(lockNodePath, new byte[0], CreateMode.PERSISTENT);
+ for (String child : zkClient.getChildren(path, null)) {
+ deleteEmptyDirs(
+ path +
DistributedCollectionConfigSetCommandRunner.ZK_PATH_SEPARATOR + child);
}
- } catch (KeeperException.NodeExistsException nee) {
- // Some other thread (on this or another JVM) beat us to create the
node, that's ok.
+ zkClient.delete(path, -1);
+ } catch (KeeperException.NoNodeException |
KeeperException.NotEmptyException e) {
+ // Already gone, or a lock is in use underneath. Either way, nothing to
do.
}
}
}
diff --git
a/solr/core/src/java/org/apache/solr/cloud/api/collections/DistributedCollectionConfigSetCommandRunner.java
b/solr/core/src/java/org/apache/solr/cloud/api/collections/DistributedCollectionConfigSetCommandRunner.java
index 77f9481b196..b9cf8bb7eda 100644
---
a/solr/core/src/java/org/apache/solr/cloud/api/collections/DistributedCollectionConfigSetCommandRunner.java
+++
b/solr/core/src/java/org/apache/solr/cloud/api/collections/DistributedCollectionConfigSetCommandRunner.java
@@ -47,6 +47,7 @@ import
org.apache.solr.cloud.ZkDistributedConfigSetLockFactory;
import org.apache.solr.common.SolrException;
import org.apache.solr.common.cloud.SolrZkClient;
import org.apache.solr.common.cloud.ZkNodeProps;
+import org.apache.solr.common.params.CollectionParams.CollectionAction;
import org.apache.solr.common.params.ConfigSetParams;
import org.apache.solr.common.params.CoreAdminParams;
import org.apache.solr.common.util.ExecutorUtil;
@@ -75,7 +76,8 @@ public class DistributedCollectionConfigSetCommandRunner {
private static final String ZK_DISTRIBUTED_API_ROOT = "/distributedapi";
/** Zookeeper node below which the locking hierarchy is anchored */
- private static final String ZK_COLLECTION_LOCKS = ZK_DISTRIBUTED_API_ROOT +
"/collectionlocks";
+ @VisibleForTesting
+ static final String ZK_COLLECTION_LOCKS = ZK_DISTRIBUTED_API_ROOT +
"/collectionlocks";
private static final String ZK_CONFIG_SET_LOCKS = ZK_DISTRIBUTED_API_ROOT +
"/configsetlocks";
@@ -400,10 +402,11 @@ public class DistributedCollectionConfigSetCommandRunner {
// creations is low, and these classes do not hold state but only
serve as an interface to
// Zookeeper. Note that after this call, we MUST execute the
lock.release(); in the finally
// below
+ ZkDistributedCollectionLockFactory lockFactory =
+ new ZkDistributedCollectionLockFactory(
+ ccc.getZkStateReader().getZkClient(), ZK_COLLECTION_LOCKS);
DistributedMultiLock lock =
- new CollectionApiLockFactory(
- new ZkDistributedCollectionLockFactory(
- ccc.getZkStateReader().getZkClient(),
ZK_COLLECTION_LOCKS))
+ new CollectionApiLockFactory(lockFactory)
.createCollectionApiLock(adminCmdContext, collName, shardId,
replicaName);
try {
@@ -448,10 +451,6 @@ public class DistributedCollectionConfigSetCommandRunner {
}
} finally {
try {
- // TODO If the Collection API command failed because the
collection does not exist,
- // we've just created some lock directory structure for a non
existent collection...
- // Maybe try to remove it here? No big deal for now as leftover
nodes in the lock
- // hierarchy do no harm, and there shouldn't be too many of those.
lock.release();
} catch (SolrException se) {
if (log.isErrorEnabled()) {
@@ -461,6 +460,9 @@ public class DistributedCollectionConfigSetCommandRunner {
se);
}
}
+ if (adminCmdContext.getAction() == CollectionAction.DELETE) {
+ deleteEmptyLockDirs(lockFactory, collName);
+ }
}
} catch (Exception e) {
if (e instanceof InterruptedException) {
@@ -477,6 +479,18 @@ public class DistributedCollectionConfigSetCommandRunner {
}
}
+ private static void deleteEmptyLockDirs(
+ ZkDistributedCollectionLockFactory lockFactory, String collName) {
+ try {
+ lockFactory.deleteEmptyLockDirs(collName);
+ } catch (Exception e) {
+ if (e instanceof InterruptedException) {
+ Thread.currentThread().interrupt();
+ }
+ log.warn("Could not delete lock dirs of deleted collection {}",
collName, e);
+ }
+ }
+
/**
* All Config Set API commands are executed in separate threads so that the
command can run to
* completion even if the client request times out.<br>
diff --git
a/solr/core/src/test/org/apache/solr/cloud/ZkDistributedLockTest.java
b/solr/core/src/test/org/apache/solr/cloud/ZkDistributedLockTest.java
index e31f042bd09..8a538ccc11a 100644
--- a/solr/core/src/test/org/apache/solr/cloud/ZkDistributedLockTest.java
+++ b/solr/core/src/test/org/apache/solr/cloud/ZkDistributedLockTest.java
@@ -17,11 +17,18 @@
package org.apache.solr.cloud;
+import java.util.List;
import java.util.concurrent.CountDownLatch;
import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicBoolean;
+import java.util.concurrent.atomic.AtomicInteger;
+import java.util.concurrent.atomic.AtomicReference;
import org.apache.solr.SolrTestCaseJ4;
+import org.apache.solr.common.SolrException;
import org.apache.solr.common.cloud.SolrZkClient;
import org.apache.solr.common.params.CollectionParams;
+import org.apache.zookeeper.CreateMode;
+import org.apache.zookeeper.KeeperException;
import org.junit.Test;
public class ZkDistributedLockTest extends SolrTestCaseJ4 {
@@ -71,6 +78,247 @@ public class ZkDistributedLockTest extends SolrTestCaseJ4 {
}
}
+ /**
+ * Lock directories must be ZooKeeper container nodes so they get removed
once empty, and a lock
+ * must still be obtainable after its directory was removed.
+ */
+ @Test
+ public void testLockDirsAreContainers() throws Exception {
+ ZkTestServer server = new ZkTestServer(createTempDir("zkData"));
+ try {
+ server.run();
+ try (SolrZkClient zkClient =
+ new SolrZkClient.Builder()
+ .withUrl(server.getZkHost())
+ .withTimeout(TIMEOUT, TimeUnit.MILLISECONDS)
+ .build()) {
+ String collRoot = "/lockTestCollectionRoot";
+ DistributedCollectionLockFactory collLockFactory =
+ new ZkDistributedCollectionLockFactory(zkClient, collRoot);
+ DistributedLock replicaLock =
+ collLockFactory.createLock(
+ true,
+ CollectionParams.LockLevel.REPLICA,
+ COLLECTION_NAME,
+ SHARD_NAME,
+ REPLICA_NAME,
+ null);
+ replicaLock.release();
+ DistributedLock shardLock =
+ collLockFactory.createLock(
+ true, CollectionParams.LockLevel.SHARD, COLLECTION_NAME,
SHARD_NAME, null, null);
+ shardLock.release();
+ DistributedLock collLock =
+ collLockFactory.createLock(
+ true, CollectionParams.LockLevel.COLLECTION, COLLECTION_NAME,
null, null, null);
+ collLock.release();
+ assertLockDirsAreContainers(server, replicaLock, collRoot);
+ assertLockDirsAreContainers(server, shardLock, collRoot);
+ assertLockDirsAreContainers(server, collLock, collRoot);
+
+ String configSetRoot = "/lockTestConfigSetRoot";
+ DistributedConfigSetLockFactory configSetLockFactory =
+ new ZkDistributedConfigSetLockFactory(zkClient, configSetRoot);
+ DistributedLock configSetLock = configSetLockFactory.createLock(true,
CONFIG_SET_NAME);
+ configSetLock.release();
+ assertLockDirsAreContainers(server, configSetLock, configSetRoot);
+
+ // Simulate ZooKeeper removing the empty containers
+ zkClient.clean(parent(parent(collLock.getLockId())));
+ DistributedLock lock =
+ collLockFactory.createLock(
+ true, CollectionParams.LockLevel.SHARD, COLLECTION_NAME,
SHARD_NAME, null, null);
+ assertTrue(lock.isAcquired());
+ lock.release();
+ }
+ } finally {
+ server.shutdown();
+ }
+ }
+
+ /** Lock dirs left as persistent nodes by older versions get removed, live
locks are kept. */
+ @Test
+ public void testDeleteEmptyLockDirs() throws Exception {
+ ZkTestServer server = new ZkTestServer(createTempDir("zkData"));
+ try {
+ server.run();
+ try (SolrZkClient zkClient =
+ new SolrZkClient.Builder()
+ .withUrl(server.getZkAddress())
+ .withTimeout(TIMEOUT, TimeUnit.MILLISECONDS)
+ .build()) {
+ String root = "/lockTestCollectionRoot";
+ ZkDistributedCollectionLockFactory factory =
+ new ZkDistributedCollectionLockFactory(zkClient, root);
+ String collLockDir =
+ lockDir(
+ factory.createLock(
+ true,
+ CollectionParams.LockLevel.COLLECTION,
+ COLLECTION_NAME,
+ null,
+ null,
+ null));
+ String replicaLockDir =
+ lockDir(
+ factory.createLock(
+ true,
+ CollectionParams.LockLevel.REPLICA,
+ COLLECTION_NAME,
+ SHARD_NAME,
+ REPLICA_NAME,
+ null));
+ String shardLockDir =
+ lockDir(
+ factory.createLock(
+ true,
+ CollectionParams.LockLevel.SHARD,
+ COLLECTION_NAME,
+ SHARD_NAME,
+ null,
+ null));
+ String otherCollLockDir =
+ lockDir(
+ factory.createLock(
+ true, CollectionParams.LockLevel.COLLECTION, "otherColl",
null, null, null));
+ String coll = parent(collLockDir);
+ zkClient.clean(root);
+ for (String dir : List.of(collLockDir, shardLockDir, replicaLockDir,
otherCollLockDir)) {
+ zkClient.makePath(dir, false);
+ }
+
+ DistributedLock shardLock =
+ factory.createLock(
+ true, CollectionParams.LockLevel.SHARD, COLLECTION_NAME,
SHARD_NAME, null, null);
+
+ factory.deleteEmptyLockDirs(COLLECTION_NAME);
+ assertFalse(zkClient.exists(collLockDir));
+ assertFalse(zkClient.exists(replicaLockDir));
+ assertTrue(zkClient.exists(shardLock.getLockId()));
+ assertTrue(shardLock.isAcquired());
+
+ shardLock.release();
+ factory.deleteEmptyLockDirs(COLLECTION_NAME);
+ assertFalse(zkClient.exists(coll));
+ assertTrue(zkClient.exists(otherCollLockDir));
+ }
+ } finally {
+ server.shutdown();
+ }
+ }
+
+ /** A parent lock dir removed while the lock path is being created must not
fail the lock. */
+ @Test
+ public void testParentRemovedWhileCreatingLockPath() throws Exception {
+ ZkTestServer server = new ZkTestServer(createTempDir("zkData"));
+ try {
+ server.run();
+ String root = "/lockTestCollectionRoot";
+ AtomicReference<String> shardLockDir = new AtomicReference<>();
+ AtomicBoolean removed = new AtomicBoolean();
+ try (SolrZkClient zkClient =
+ new SolrZkClient(
+ new SolrZkClient.Builder()
+ .withUrl(server.getZkAddress())
+ .withTimeout(TIMEOUT, TimeUnit.MILLISECONDS)) {
+ @Override
+ public String create(String path, byte[] data, CreateMode
createMode)
+ throws KeeperException, InterruptedException {
+ if (path.equals(shardLockDir.get()) &&
removed.compareAndSet(false, true)) {
+ delete(parent(path), -1);
+ }
+ return super.create(path, data, createMode);
+ }
+ }) {
+ ZkDistributedCollectionLockFactory factory =
+ new ZkDistributedCollectionLockFactory(zkClient, root);
+ shardLockDir.set(
+ lockDir(
+ factory.createLock(
+ true,
+ CollectionParams.LockLevel.SHARD,
+ COLLECTION_NAME,
+ SHARD_NAME,
+ null,
+ null)));
+ zkClient.clean(root);
+
+ DistributedLock lock =
+ factory.createLock(
+ true, CollectionParams.LockLevel.SHARD, COLLECTION_NAME,
SHARD_NAME, null, null);
+ assertTrue(removed.get());
+ assertTrue(lock.isAcquired());
+ lock.release();
+ }
+ } finally {
+ server.shutdown();
+ }
+ }
+
+ /** Lock creation gives up once the lock dir keeps disappearing on every
attempt. */
+ @Test
+ public void testLockCreationGivesUpAfterMaxAttempts() throws Exception {
+ ZkTestServer server = new ZkTestServer(createTempDir("zkData"));
+ try {
+ server.run();
+ AtomicInteger lockNodeCreates = new AtomicInteger();
+ try (SolrZkClient zkClient =
+ new SolrZkClient(
+ new SolrZkClient.Builder()
+ .withUrl(server.getZkAddress())
+ .withTimeout(TIMEOUT, TimeUnit.MILLISECONDS)) {
+ @Override
+ public String create(String path, byte[] data, CreateMode
createMode)
+ throws KeeperException, InterruptedException {
+ if (createMode == CreateMode.EPHEMERAL_SEQUENTIAL
+ && lockNodeCreates.incrementAndGet()
+ <= ZkDistributedLockFactory.MAX_CREATE_ATTEMPTS) {
+ throw new KeeperException.NoNodeException(path);
+ }
+ return super.create(path, data, createMode);
+ }
+ }) {
+ ZkDistributedCollectionLockFactory factory =
+ new ZkDistributedCollectionLockFactory(zkClient,
"/lockTestCollectionRoot");
+ SolrException e =
+ expectThrows(
+ SolrException.class,
+ () ->
+ factory.createLock(
+ true,
+ CollectionParams.LockLevel.COLLECTION,
+ COLLECTION_NAME,
+ null,
+ null,
+ null));
+ assertTrue(e.getCause() instanceof KeeperException.NoNodeException);
+ assertEquals(ZkDistributedLockFactory.MAX_CREATE_ATTEMPTS,
lockNodeCreates.get());
+ }
+ } finally {
+ server.shutdown();
+ }
+ }
+
+ /** Asserts every node between the lock and {@code root} is a container. */
+ private static void assertLockDirsAreContainers(
+ ZkTestServer server, DistributedLock lock, String root) {
+ for (String dir = parent(lock.getLockId()); !dir.equals(root); dir =
parent(dir)) {
+ assertTrue(
+ dir + " should be a container node",
+ server.getZKDatabase().getDataTree().getContainers().contains(dir));
+ }
+ }
+
+ /** Releases the lock and returns the dir it was created in. */
+ private static String lockDir(DistributedLock lock) {
+ lock.release();
+ return parent(lock.getLockId());
+ }
+
+ private static String parent(String path) {
+ return path.substring(0, path.lastIndexOf('/'));
+ }
+
private void monothreadedCollectionTests(DistributedCollectionLockFactory
factory) {
// Collection level locks
DistributedLock collRL1 =
diff --git
a/solr/core/src/test/org/apache/solr/cloud/api/collections/DeleteCollectionLockCleanupTest.java
b/solr/core/src/test/org/apache/solr/cloud/api/collections/DeleteCollectionLockCleanupTest.java
new file mode 100644
index 00000000000..63977657a8e
--- /dev/null
+++
b/solr/core/src/test/org/apache/solr/cloud/api/collections/DeleteCollectionLockCleanupTest.java
@@ -0,0 +1,79 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.solr.cloud.api.collections;
+
+import java.util.List;
+import org.apache.solr.client.solrj.request.CollectionAdminRequest;
+import org.apache.solr.cloud.DistributedLock;
+import org.apache.solr.cloud.SolrCloudTestCase;
+import org.apache.solr.cloud.ZkDistributedCollectionLockFactory;
+import org.apache.solr.common.cloud.SolrZkClient;
+import org.apache.solr.common.params.CollectionParams.LockLevel;
+import org.junit.BeforeClass;
+import org.junit.Test;
+
+/**
+ * With the Overseer disabled, deleting a collection removes the persistent
lock dirs that older
+ * versions left for it.
+ */
+public class DeleteCollectionLockCleanupTest extends SolrCloudTestCase {
+
+ @BeforeClass
+ public static void setupCluster() throws Exception {
+ configureCluster(1)
+ .addConfig("conf", configset("cloud-minimal"))
+ .withOverseer(false)
+ .configure();
+ }
+
+ @Test
+ public void testDeleteRemovesLegacyLockDirs() throws Exception {
+ String collection = "lockCleanup";
+ CollectionAdminRequest.createCollection(collection, "conf", 1, 1)
+ .process(cluster.getSolrClient());
+ cluster.waitForActiveCollection(collection, 1, 1);
+ String shard =
getCollectionState(collection).getSlices().iterator().next().getName();
+
+ SolrZkClient zkClient = cluster.getZkClient();
+ ZkDistributedCollectionLockFactory factory =
+ new ZkDistributedCollectionLockFactory(
+ zkClient,
DistributedCollectionConfigSetCommandRunner.ZK_COLLECTION_LOCKS);
+ String collLockDir =
+ lockDir(factory.createLock(true, LockLevel.COLLECTION, collection,
null, null, null));
+ String shardLockDir =
+ lockDir(factory.createLock(true, LockLevel.SHARD, collection, shard,
null, null));
+ String collDir = parent(collLockDir);
+
+ zkClient.clean(collDir);
+ for (String dir : List.of(collLockDir, shardLockDir)) {
+ zkClient.makePath(dir, false);
+ }
+
+
CollectionAdminRequest.deleteCollection(collection).process(cluster.getSolrClient());
+ assertFalse(collDir + " should have been removed",
zkClient.exists(collDir));
+ }
+
+ private static String lockDir(DistributedLock lock) {
+ lock.release();
+ return parent(lock.getLockId());
+ }
+
+ private static String parent(String path) {
+ return path.substring(0, path.lastIndexOf('/'));
+ }
+}
diff --git
a/solr/solr-ref-guide/modules/upgrade-notes/pages/major-changes-in-solr-10.adoc
b/solr/solr-ref-guide/modules/upgrade-notes/pages/major-changes-in-solr-10.adoc
index 37a82e00720..a98410083a7 100644
---
a/solr/solr-ref-guide/modules/upgrade-notes/pages/major-changes-in-solr-10.adoc
+++
b/solr/solr-ref-guide/modules/upgrade-notes/pages/major-changes-in-solr-10.adoc
@@ -237,6 +237,8 @@ All nodes in the cluster must always have a consistent
understanding of the over
If using the cluster property toggle, use the
xref:deployment-guide:zookeeper-utilities.adoc#set-a-cluster-property[bin/solr
cluster] CLI utility to set it while the cluster is offline.
If using the env var; ensure each Solr node is configured to start with the
setting set consistently.
+NOTE: Without the Overseer, Solr keeps its Collection and ConfigSet API locks
in ZooKeeper container znodes, so an external ZooKeeper ensemble must be
version 3.5.1 or higher.
+
When the Overseer is disabled, it is nonetheless still elected, which can be
influenced by node roles.
If you are using any
xref:configuration-guide:cluster-singleton-plugins.adoc[cluster singleton
plugins], they execute on the node elected to be the Overseer.
