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 6518c15fbb5e372cf0e636b331699eb67248e833 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" (cherry picked from commit 0d48fb5b1afdc985f1577576d32deb3d649a418e) --- .../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.
