From 33cf936b530e1c7d9115d4ec724aaaa8f7d2e83e Mon Sep 17 00:00:00 2001 From: Daniel Roberts ddanielr Date: Tue, 3 Mar 2026 23:10:35 +0000 Subject: [PATCH 1/3] Fix gc lock removals Does not delete the top level `gc/lock/` path, instead it just deletes all locks underneath the path. Updates ITs and the Admin utility to delete gc locks the same way. --- .../core/fate/zookeeper/ServiceLock.java | 17 ++++++++++++++--- .../org/apache/accumulo/server/util/Admin.java | 4 +++- .../test/functional/GarbageCollectorIT.java | 4 ++-- .../test/upgrade/GCUpgrade9to10TestIT.java | 6 ++++-- 4 files changed, 23 insertions(+), 8 deletions(-) diff --git a/core/src/main/java/org/apache/accumulo/core/fate/zookeeper/ServiceLock.java b/core/src/main/java/org/apache/accumulo/core/fate/zookeeper/ServiceLock.java index b0645ea8a23..0d6a3fc91a1 100644 --- a/core/src/main/java/org/apache/accumulo/core/fate/zookeeper/ServiceLock.java +++ b/core/src/main/java/org/apache/accumulo/core/fate/zookeeper/ServiceLock.java @@ -825,13 +825,24 @@ public static void deleteLock(ZooReaderWriter zk, ServiceLockPath path) public static void deleteLock(ZooReaderWriter zoo, String path, ServerServices.Service serviceType, Predicate hostPortPredicate, Consumer messageOutput, Boolean dryRun) throws KeeperException, InterruptedException { + + Objects.requireNonNull(path, "Lock path cannot be null"); + Objects.requireNonNull(hostPortPredicate, "host predicate cannot be null"); + + if (!zoo.exists(path)) { + throw new IllegalStateException("Path " + path + " does not exist"); + } + var lockData = ServiceLock.getLockData(zoo.getZooKeeper(), ServiceLock.path(path)); if (lockData != null) { ServerServices lock = new ServerServices(new String(lockData, UTF_8)); if (hostPortPredicate.test(lock.getAddress(serviceType))) { - messageOutput.accept("Deleting " + path + " from zookeeper"); - if (!dryRun) { - zoo.recursiveDelete(path, NodeMissingPolicy.SKIP); + List children = zoo.getChildren(path); + for (String child : children) { + messageOutput.accept("Deleting " + path + "/" + child + " from zookeeper"); + if (!dryRun) { + zoo.recursiveDelete(path + "/" + child, NodeMissingPolicy.SKIP); + } } } } diff --git a/server/base/src/main/java/org/apache/accumulo/server/util/Admin.java b/server/base/src/main/java/org/apache/accumulo/server/util/Admin.java index b081922e6ff..b1eae021d64 100644 --- a/server/base/src/main/java/org/apache/accumulo/server/util/Admin.java +++ b/server/base/src/main/java/org/apache/accumulo/server/util/Admin.java @@ -75,6 +75,7 @@ import org.apache.accumulo.core.trace.TraceUtil; import org.apache.accumulo.core.util.AddressUtil; import org.apache.accumulo.core.util.HostAndPort; +import org.apache.accumulo.core.util.ServerServices; import org.apache.accumulo.core.util.tables.TableMap; import org.apache.accumulo.server.ServerContext; import org.apache.accumulo.server.cli.ServerUtilOpts; @@ -600,7 +601,8 @@ private static void stopServers(final ServerContext context, List server String managerLockPath = Constants.ZROOT + "/" + iid + Constants.ZMANAGER_LOCK; ZooZap.removeSingletonLock(zk, managerLockPath, hostAndPort::contains, opts); String gcLockPath = Constants.ZROOT + "/" + iid + Constants.ZGC_LOCK; - ZooZap.removeSingletonLock(zk, gcLockPath, hostAndPort::contains, opts); + ServiceLock.deleteLock(zk, gcLockPath, ServerServices.Service.GC_CLIENT, + hostAndPort::contains, log::debug, opts.dryRun); String monitorLockPath = Constants.ZROOT + "/" + iid + Constants.ZMONITOR_LOCK; ZooZap.removeSingletonLock(zk, monitorLockPath, hostAndPort::contains, opts); } else { diff --git a/test/src/main/java/org/apache/accumulo/test/functional/GarbageCollectorIT.java b/test/src/main/java/org/apache/accumulo/test/functional/GarbageCollectorIT.java index 25e89fbd60d..343eade3545 100644 --- a/test/src/main/java/org/apache/accumulo/test/functional/GarbageCollectorIT.java +++ b/test/src/main/java/org/apache/accumulo/test/functional/GarbageCollectorIT.java @@ -118,10 +118,10 @@ private void killMacGc() throws ProcessNotFoundException, InterruptedException, getCluster().killProcess(ServerType.GARBAGE_COLLECTOR, getCluster().getProcesses().get(ServerType.GARBAGE_COLLECTOR).iterator().next()); // delete lock in zookeeper if there, this will allow next GC to start quickly - var path = ServiceLock.path(getServerContext().getZooKeeperRoot() + Constants.ZGC_LOCK); + String path = getServerContext().getZooKeeperRoot() + Constants.ZGC_LOCK; ZooReaderWriter zk = getServerContext().getZooReaderWriter(); try { - ServiceLock.deleteLock(zk, path); + ServiceLock.deleteLock(zk, path, Service.GC_CLIENT, hostAndPort -> true, log::debug, false); } catch (IllegalStateException e) { log.error("Unable to delete ZooLock for mini accumulo-gc", e); } diff --git a/test/src/main/java/org/apache/accumulo/test/upgrade/GCUpgrade9to10TestIT.java b/test/src/main/java/org/apache/accumulo/test/upgrade/GCUpgrade9to10TestIT.java index 897750022ed..15cfbd1acef 100644 --- a/test/src/main/java/org/apache/accumulo/test/upgrade/GCUpgrade9to10TestIT.java +++ b/test/src/main/java/org/apache/accumulo/test/upgrade/GCUpgrade9to10TestIT.java @@ -46,6 +46,7 @@ import org.apache.accumulo.core.metadata.schema.MetadataSchema.DeletesSection; import org.apache.accumulo.core.security.Authorizations; import org.apache.accumulo.core.security.TablePermission; +import org.apache.accumulo.core.util.ServerServices; import org.apache.accumulo.manager.upgrade.Upgrader9to10; import org.apache.accumulo.minicluster.ServerType; import org.apache.accumulo.miniclusterImpl.MiniAccumuloConfigImpl; @@ -84,10 +85,11 @@ private void killMacGc() throws ProcessNotFoundException, InterruptedException, getCluster().killProcess(ServerType.GARBAGE_COLLECTOR, getCluster().getProcesses().get(ServerType.GARBAGE_COLLECTOR).iterator().next()); // delete lock in zookeeper if there, this will allow next GC to start quickly - var path = ServiceLock.path(getServerContext().getZooKeeperRoot() + Constants.ZGC_LOCK); + String path = getServerContext().getZooKeeperRoot() + Constants.ZGC_LOCK; ZooReaderWriter zk = getServerContext().getZooReaderWriter(); try { - ServiceLock.deleteLock(zk, path); + ServiceLock.deleteLock(zk, path, ServerServices.Service.GC_CLIENT, hostAndPort -> true, + log::debug, false); } catch (IllegalStateException e) { log.error("Unable to delete ZooLock for mini accumulo-gc", e); } From e35c97def50a828a6471d7f0e91f0f559f4338ee Mon Sep 17 00:00:00 2001 From: Daniel Roberts ddanielr Date: Wed, 4 Mar 2026 18:15:45 +0000 Subject: [PATCH 2/3] Improvements to Admin stop command Switches to using ServiceLock.deleteLocks for tserver locks so lock deletes can show up in the Admin log (if debug is enabled). Fixed a bug where the command would fail if no scan servers were running. Added the ability to also force stop the compaction-coordinator server --- .../java/org/apache/accumulo/server/util/Admin.java | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/server/base/src/main/java/org/apache/accumulo/server/util/Admin.java b/server/base/src/main/java/org/apache/accumulo/server/util/Admin.java index b1eae021d64..8251333ee23 100644 --- a/server/base/src/main/java/org/apache/accumulo/server/util/Admin.java +++ b/server/base/src/main/java/org/apache/accumulo/server/util/Admin.java @@ -590,13 +590,17 @@ private static void stopServers(final ServerContext context, List server var iid = context.getInstanceID(); String tserversPath = Constants.ZROOT + "/" + iid + Constants.ZTSERVERS; - ZooZap.removeLocks(zk, tserversPath, hostAndPort::contains, opts); + ServiceLock.deleteLocks(zk, tserversPath, hostAndPort::contains, log::debug, false); String compactorsBasepath = Constants.ZROOT + "/" + iid + Constants.ZCOMPACTORS; ZooZap.removeCompactorGroupedLocks(zk, compactorsBasepath, rg -> true, hostAndPort::contains, opts); String sserversPath = Constants.ZROOT + "/" + iid + Constants.ZSSERVERS; - ZooZap.removeScanServerGroupLocks(zk, sserversPath, hostAndPort::contains, rg -> true, - opts); + try { + ZooZap.removeScanServerGroupLocks(zk, sserversPath, hostAndPort::contains, rg -> true, + opts); + } catch (IllegalStateException e) { + log.debug("No Scan Server locks currently exist", e); + } String managerLockPath = Constants.ZROOT + "/" + iid + Constants.ZMANAGER_LOCK; ZooZap.removeSingletonLock(zk, managerLockPath, hostAndPort::contains, opts); @@ -605,6 +609,9 @@ private static void stopServers(final ServerContext context, List server hostAndPort::contains, log::debug, opts.dryRun); String monitorLockPath = Constants.ZROOT + "/" + iid + Constants.ZMONITOR_LOCK; ZooZap.removeSingletonLock(zk, monitorLockPath, hostAndPort::contains, opts); + String compactionCoordinatorLockPath = + Constants.ZROOT + "/" + iid + Constants.ZCOORDINATOR_LOCK; + ZooZap.removeSingletonLock(zk, compactionCoordinatorLockPath, hostAndPort::contains, opts); } else { for (var server : hostAndPort) { signalGracefulShutdown(context, server); From 31dc2357cd333caba405a7f7b297bc2bd659bac3 Mon Sep 17 00:00:00 2001 From: Daniel Roberts ddanielr Date: Wed, 4 Mar 2026 18:18:17 +0000 Subject: [PATCH 3/3] Remove duplicate exists check getLockData() also throws NoNodeException if nothing exists. --- .../org/apache/accumulo/core/fate/zookeeper/ServiceLock.java | 4 ---- 1 file changed, 4 deletions(-) diff --git a/core/src/main/java/org/apache/accumulo/core/fate/zookeeper/ServiceLock.java b/core/src/main/java/org/apache/accumulo/core/fate/zookeeper/ServiceLock.java index 0d6a3fc91a1..0d433d55c27 100644 --- a/core/src/main/java/org/apache/accumulo/core/fate/zookeeper/ServiceLock.java +++ b/core/src/main/java/org/apache/accumulo/core/fate/zookeeper/ServiceLock.java @@ -829,10 +829,6 @@ public static void deleteLock(ZooReaderWriter zoo, String path, Objects.requireNonNull(path, "Lock path cannot be null"); Objects.requireNonNull(hostPortPredicate, "host predicate cannot be null"); - if (!zoo.exists(path)) { - throw new IllegalStateException("Path " + path + " does not exist"); - } - var lockData = ServiceLock.getLockData(zoo.getZooKeeper(), ServiceLock.path(path)); if (lockData != null) { ServerServices lock = new ServerServices(new String(lockData, UTF_8));