From 859cda2ceab8be4c325e7e64fd4dc57822760c61 Mon Sep 17 00:00:00 2001 From: Dave Marion Date: Thu, 22 Jan 2026 21:00:54 +0000 Subject: [PATCH 1/7] Added ZooZap and ServiceLock to delete ScanServer locks The ScanServer locks in 2.1.x have a different format than the other locks and require different methods. The lock structure has been normalized in the planned version 4.0. Closes #6067 Co-authored-by: Dom G. --- .../core/fate/zookeeper/ServiceLock.java | 33 ++++++ .../apache/accumulo/server/util/Admin.java | 4 +- .../apache/accumulo/server/util/ZooZap.java | 10 +- .../accumulo/server/util/AdminTest.java | 106 ++++++++++++++++++ .../apache/accumulo/tserver/ScanServer.java | 2 +- 5 files changed, 152 insertions(+), 3 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 42e57206e58..cea5ee1e29a 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 @@ -18,6 +18,7 @@ */ package org.apache.accumulo.core.fate.zookeeper; +import static java.nio.charset.StandardCharsets.UTF_8; import static java.util.Objects.requireNonNull; import java.util.ArrayList; @@ -754,6 +755,38 @@ public static void deleteLocks(ZooReaderWriter zk, String zPath, } } + @Deprecated(since = "2.1.5") + public static void deleteScanServerLocks(ZooReaderWriter zk, String zPath, + Predicate hostPortPredicate, Predicate groupPredicate, + Consumer messageOutput, Boolean dryRun) throws KeeperException, InterruptedException { + + Objects.requireNonNull(zPath, "Lock path cannot be null"); + Objects.requireNonNull(groupPredicate, "group predicate cannot be null"); + if (!zk.exists(zPath)) { + throw new IllegalStateException("Path " + zPath + " does not exist"); + } + + List servers = zk.getChildren(zPath); + if (servers.isEmpty()) { + throw new IllegalStateException("No server locks are held at " + zPath); + } + + for (String server : servers) { + if (hostPortPredicate.test(HostAndPort.fromString(server))) { + byte[] lockData = zk.getData(zPath + "/" + server); + String lockContent = new String(lockData, UTF_8); + String[] parts = lockContent.split(","); + if (parts.length == 2 && groupPredicate.test(parts[1])) { + messageOutput.accept("Deleting " + zPath + "/" + server + " from zookeeper"); + if (!dryRun) { + LOG.debug("Deleting all locks at path {} due to lock deletion", zPath); + zk.recursiveDelete(zPath + "/" + server, NodeMissingPolicy.SKIP); + } + } + } + } + } + /** * This method will delete the top server lock for a given lock path * 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 c88b919ea8e..fc27501de29 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 @@ -560,6 +560,7 @@ private static void stopServer(final ClientContext context, final boolean tablet client -> client.shutdown(TraceUtil.traceInfo(), context.rpcCreds(), tabletServersToo)); } + @SuppressWarnings("deprecation") private static void stopServers(final ServerContext context, List servers, final boolean force) throws AccumuloException, AccumuloSecurityException, InterruptedException, KeeperException { @@ -592,7 +593,8 @@ private static void stopServers(final ServerContext context, List server String compactorsBasepath = Constants.ZROOT + "/" + iid + Constants.ZCOMPACTORS; ZooZap.removeGroupedLocks(zk, compactorsBasepath, rg -> true, hostAndPort::contains, opts); String sserversPath = Constants.ZROOT + "/" + iid + Constants.ZSSERVERS; - ZooZap.removeGroupedLocks(zk, sserversPath, rg -> true, hostAndPort::contains, opts); + ZooZap.removeScanServerGroupLocks(zk, sserversPath, hostAndPort::contains, rg -> true, + opts); String managerLockPath = Constants.ZROOT + "/" + iid + Constants.ZMANAGER_LOCK; ZooZap.removeSingletonLock(zk, managerLockPath, hostAndPort::contains, opts); diff --git a/server/base/src/main/java/org/apache/accumulo/server/util/ZooZap.java b/server/base/src/main/java/org/apache/accumulo/server/util/ZooZap.java index 3f8d1c0d6b0..7f635efba6d 100644 --- a/server/base/src/main/java/org/apache/accumulo/server/util/ZooZap.java +++ b/server/base/src/main/java/org/apache/accumulo/server/util/ZooZap.java @@ -245,7 +245,7 @@ public void zap(SiteConfiguration siteConf, String... args) { if (opts.zapScanServers) { String sserversPath = Constants.ZROOT + "/" + iid + Constants.ZSSERVERS; try { - removeGroupedLocks(zoo, sserversPath, groupPredicate, hostPortPredicate, opts); + removeScanServerGroupLocks(zoo, sserversPath, hostPortPredicate, groupPredicate, opts); } catch (KeeperException | InterruptedException e) { log.error("Error deleting scan server locks", e); } @@ -283,6 +283,14 @@ static void removeLocks(ZooReaderWriter zoo, String path, ServiceLock.deleteLocks(zoo, path, hostPortPredicate, m -> message(m, opts), opts.dryRun); } + @Deprecated(since = "2.1.5") + static void removeScanServerGroupLocks(ZooReaderWriter zoo, String path, + Predicate hostPortPredicate, Predicate groupPredicate, Opts opts) + throws KeeperException, InterruptedException { + ServiceLock.deleteScanServerLocks(zoo, path, hostPortPredicate, groupPredicate, + m -> message(m, opts), opts.dryRun); + } + static void removeSingletonLock(ZooReaderWriter zoo, String path, Predicate hostPortPredicate, Opts ops) throws KeeperException, InterruptedException { diff --git a/server/base/src/test/java/org/apache/accumulo/server/util/AdminTest.java b/server/base/src/test/java/org/apache/accumulo/server/util/AdminTest.java index 2e6754176e9..05206df481b 100644 --- a/server/base/src/test/java/org/apache/accumulo/server/util/AdminTest.java +++ b/server/base/src/test/java/org/apache/accumulo/server/util/AdminTest.java @@ -18,16 +18,24 @@ */ package org.apache.accumulo.server.util; +import static java.nio.charset.StandardCharsets.UTF_8; +import static org.junit.jupiter.api.Assertions.assertAll; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; import java.util.Collections; +import java.util.List; import java.util.UUID; +import java.util.concurrent.atomic.AtomicBoolean; import org.apache.accumulo.core.Constants; import org.apache.accumulo.core.clientImpl.ClientContext; import org.apache.accumulo.core.data.InstanceId; import org.apache.accumulo.core.fate.zookeeper.ZooCache; import org.apache.accumulo.core.fate.zookeeper.ZooCache.ZcStat; +import org.apache.accumulo.core.fate.zookeeper.ZooReaderWriter; +import org.apache.accumulo.core.fate.zookeeper.ZooUtil.NodeMissingPolicy; import org.easymock.EasyMock; import org.junit.jupiter.api.Test; @@ -93,4 +101,102 @@ public void testCannotQualifySessionId() { EasyMock.verify(zc); } + /** + * SServer group filter should use lock data (UUID,group). + */ + @SuppressWarnings("deprecation") + @Test + public void testSserverGroupFilterUsesLockData() throws Exception { + ZooReaderWriter zoo = EasyMock.createNiceMock(ZooReaderWriter.class); + + String basePath = "/accumulo/iid/sservers"; + String hostDefault = "host1:10000"; + String hostOther = "host2:10001"; + + EasyMock.expect(zoo.exists(basePath)).andReturn(true); + EasyMock.expect(zoo.getChildren(basePath)).andReturn(List.of(hostDefault, hostOther)); + EasyMock.expect(zoo.getData(basePath + "/" + hostDefault)) + .andReturn((UUID.randomUUID().toString() + ",default").getBytes(UTF_8)); + EasyMock.expect(zoo.getData(basePath + "/" + hostOther)) + .andReturn((UUID.randomUUID().toString() + ",rg1").getBytes(UTF_8)); + + AtomicBoolean deletedDefault = new AtomicBoolean(false); + AtomicBoolean deletedOther = new AtomicBoolean(false); + + zoo.recursiveDelete(basePath + "/" + hostDefault, NodeMissingPolicy.SKIP); + EasyMock.expectLastCall().andStubAnswer(() -> { + deletedDefault.set(true); + return null; + }); + + zoo.recursiveDelete(basePath + "/" + hostOther, NodeMissingPolicy.SKIP); + EasyMock.expectLastCall().andStubAnswer(() -> { + deletedOther.set(true); + return null; + }); + + EasyMock.replay(zoo); + + ZooZap.Opts opts = new ZooZap.Opts(); + ZooZap.removeScanServerGroupLocks(zoo, basePath, hp -> true, "default"::equals, opts); + assertAll(() -> { + assertTrue(deletedDefault.get(), + "Expected scan server lock for group 'default' to be deleted, but it was not."); + assertFalse(deletedOther.get(), + "Expected scan server lock for group 'rg1' to be preserved, but it was deleted."); + }); + } + + /** + * SServer cleanup without group filter should delete all host nodes. + */ + @SuppressWarnings("deprecation") + @Test + public void testSserverDeleteAllNoGroupFilter() throws Exception { + ZooReaderWriter zoo = EasyMock.createNiceMock(ZooReaderWriter.class); + + String basePath = "/accumulo/iid/sservers"; + String host1 = "host1:10000"; + String host2 = "host2:10001"; + String lock1 = "zlock#" + UUID.randomUUID() + "#0000000000"; + String lock2 = "zlock#" + UUID.randomUUID() + "#0000000000"; + + EasyMock.expect(zoo.exists(basePath)).andReturn(true); + EasyMock.expect(zoo.getChildren(basePath)).andReturn(List.of(host1, host2)); + EasyMock.expect(zoo.getData(basePath + "/" + host1)) + .andReturn((UUID.randomUUID().toString() + ",default").getBytes(UTF_8)); + EasyMock.expect(zoo.getData(basePath + "/" + host2)) + .andReturn((UUID.randomUUID().toString() + ",rg1").getBytes(UTF_8)); + + EasyMock.expect(zoo.exists(basePath + "/" + host1)).andReturn(true); + EasyMock.expect(zoo.getChildren(basePath + "/" + host1)).andReturn(List.of(lock1)); + AtomicBoolean deletedHost1 = new AtomicBoolean(false); + AtomicBoolean deletedHost2 = new AtomicBoolean(false); + + zoo.recursiveDelete(basePath + "/" + host1, NodeMissingPolicy.SKIP); + EasyMock.expectLastCall().andStubAnswer(() -> { + deletedHost1.set(true); + return null; + }); + + EasyMock.expect(zoo.exists(basePath + "/" + host2)).andReturn(true); + EasyMock.expect(zoo.getChildren(basePath + "/" + host2)).andReturn(List.of(lock2)); + zoo.recursiveDelete(basePath + "/" + host2, NodeMissingPolicy.SKIP); + EasyMock.expectLastCall().andStubAnswer(() -> { + deletedHost2.set(true); + return null; + }); + + EasyMock.replay(zoo); + + ZooZap.Opts opts = new ZooZap.Opts(); + ZooZap.removeScanServerGroupLocks(zoo, basePath, hp -> true, g -> true, opts); + + assertAll(() -> { + assertTrue(deletedHost1.get(), + "Expected scan server lock for host1 to be deleted when no group filter is set."); + assertTrue(deletedHost2.get(), + "Expected scan server lock for host2 to be deleted when no group filter is set."); + }); + } } diff --git a/server/tserver/src/main/java/org/apache/accumulo/tserver/ScanServer.java b/server/tserver/src/main/java/org/apache/accumulo/tserver/ScanServer.java index ec89e668af9..83f57347b16 100644 --- a/server/tserver/src/main/java/org/apache/accumulo/tserver/ScanServer.java +++ b/server/tserver/src/main/java/org/apache/accumulo/tserver/ScanServer.java @@ -328,7 +328,7 @@ public String getClientAddressString() { } /** - * Set up nodes and locks in ZooKeeper for this Compactor + * Set up nodes and locks in ZooKeeper for this ScanServer */ private ServiceLock announceExistence() { ZooReaderWriter zoo = getContext().getZooReaderWriter(); From 809f3bcf20cc178c222cb64927f20275620fcbb6 Mon Sep 17 00:00:00 2001 From: Dave Marion Date: Thu, 22 Jan 2026 21:49:38 +0000 Subject: [PATCH 2/7] Fix issue in accumulo-cluster script with json and jq --- assemble/bin/accumulo-cluster | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/assemble/bin/accumulo-cluster b/assemble/bin/accumulo-cluster index 6e8b4e0df8f..dc7711fd269 100755 --- a/assemble/bin/accumulo-cluster +++ b/assemble/bin/accumulo-cluster @@ -796,7 +796,7 @@ function prune() { read -r -a groups <<<"$ARG_SSERVER_GROUP" else # find all groups known in zookeeper, this will allow pruning entire groups that do not even exist in cluster.yaml - readarray -t groups < <(jq -r ".summaries.S_SERVER.resourceGroups | .[] " "$service_json") + readarray -t groups < <(jq -r ".summaries.S_SERVER.resourceGroups | keys | .[]?" "$service_json") fi for group in "${groups[@]}"; do @@ -815,7 +815,7 @@ function prune() { read -r -a groups <<<"$ARG_COMPACTOR_GROUP" else # find all groups known in zookeeper, this will allow pruning entire groups that do not even exist in cluster.yaml - readarray -t groups < <(jq -r ".summaries.COMPACTOR.resourceGroups | .[] " "$service_json") + readarray -t groups < <(jq -r ".summaries.COMPACTOR.resourceGroups | keys | .[]?" "$service_json") fi for group in "${groups[@]}"; do From 5a33208dc6ba63230bc6cc4d6dddad92a18a4155 Mon Sep 17 00:00:00 2001 From: Dave Marion Date: Thu, 22 Jan 2026 22:59:44 +0000 Subject: [PATCH 3/7] Changes from testing --- .../core/fate/zookeeper/ServiceLock.java | 3 +- .../accumulo/server/util/AdminTest.java | 37 +++++++++++++------ 2 files changed, 27 insertions(+), 13 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 cea5ee1e29a..d072ea520ed 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 @@ -771,9 +771,10 @@ public static void deleteScanServerLocks(ZooReaderWriter zk, String zPath, throw new IllegalStateException("No server locks are held at " + zPath); } + ZooKeeper z = zk.getZooKeeper(); for (String server : servers) { if (hostPortPredicate.test(HostAndPort.fromString(server))) { - byte[] lockData = zk.getData(zPath + "/" + server); + byte[] lockData = ServiceLock.getLockData(z, path(zPath + "/" + server)); String lockContent = new String(lockData, UTF_8); String[] parts = lockContent.split(","); if (parts.length == 2 && groupPredicate.test(parts[1])) { diff --git a/server/base/src/test/java/org/apache/accumulo/server/util/AdminTest.java b/server/base/src/test/java/org/apache/accumulo/server/util/AdminTest.java index 05206df481b..a86b8b16f8c 100644 --- a/server/base/src/test/java/org/apache/accumulo/server/util/AdminTest.java +++ b/server/base/src/test/java/org/apache/accumulo/server/util/AdminTest.java @@ -36,6 +36,7 @@ import org.apache.accumulo.core.fate.zookeeper.ZooCache.ZcStat; import org.apache.accumulo.core.fate.zookeeper.ZooReaderWriter; import org.apache.accumulo.core.fate.zookeeper.ZooUtil.NodeMissingPolicy; +import org.apache.zookeeper.ZooKeeper; import org.easymock.EasyMock; import org.junit.jupiter.api.Test; @@ -107,17 +108,24 @@ public void testCannotQualifySessionId() { @SuppressWarnings("deprecation") @Test public void testSserverGroupFilterUsesLockData() throws Exception { + ZooReaderWriter zoo = EasyMock.createNiceMock(ZooReaderWriter.class); + ZooKeeper zk = EasyMock.createNiceMock(ZooKeeper.class); String basePath = "/accumulo/iid/sservers"; String hostDefault = "host1:10000"; String hostOther = "host2:10001"; + String zlock1 = "zlock#00000000-0000-0000-0000-aaaaaaaaaaaa#0000000001"; + String zlock2 = "zlock#00000000-0000-0000-0000-bbbbbbbbbbbb#0000000001"; EasyMock.expect(zoo.exists(basePath)).andReturn(true); EasyMock.expect(zoo.getChildren(basePath)).andReturn(List.of(hostDefault, hostOther)); - EasyMock.expect(zoo.getData(basePath + "/" + hostDefault)) + EasyMock.expect(zoo.getZooKeeper()).andReturn(zk); + EasyMock.expect(zk.getChildren(basePath + "/" + hostDefault, null)).andReturn(List.of(zlock1)); + EasyMock.expect(zk.getData(basePath + "/" + hostDefault + "/" + zlock1, false, null)) .andReturn((UUID.randomUUID().toString() + ",default").getBytes(UTF_8)); - EasyMock.expect(zoo.getData(basePath + "/" + hostOther)) + EasyMock.expect(zk.getChildren(basePath + "/" + hostOther, null)).andReturn(List.of(zlock2)); + EasyMock.expect(zk.getData(basePath + "/" + hostOther + "/" + zlock2, false, null)) .andReturn((UUID.randomUUID().toString() + ",rg1").getBytes(UTF_8)); AtomicBoolean deletedDefault = new AtomicBoolean(false); @@ -135,7 +143,7 @@ public void testSserverGroupFilterUsesLockData() throws Exception { return null; }); - EasyMock.replay(zoo); + EasyMock.replay(zoo, zk); ZooZap.Opts opts = new ZooZap.Opts(); ZooZap.removeScanServerGroupLocks(zoo, basePath, hp -> true, "default"::equals, opts); @@ -145,6 +153,9 @@ public void testSserverGroupFilterUsesLockData() throws Exception { assertFalse(deletedOther.get(), "Expected scan server lock for group 'rg1' to be preserved, but it was deleted."); }); + + EasyMock.verify(zoo, zk); + } /** @@ -154,22 +165,24 @@ public void testSserverGroupFilterUsesLockData() throws Exception { @Test public void testSserverDeleteAllNoGroupFilter() throws Exception { ZooReaderWriter zoo = EasyMock.createNiceMock(ZooReaderWriter.class); + ZooKeeper zk = EasyMock.createNiceMock(ZooKeeper.class); String basePath = "/accumulo/iid/sservers"; String host1 = "host1:10000"; String host2 = "host2:10001"; - String lock1 = "zlock#" + UUID.randomUUID() + "#0000000000"; - String lock2 = "zlock#" + UUID.randomUUID() + "#0000000000"; + String zlock1 = "zlock#00000000-0000-0000-0000-aaaaaaaaaaaa#0000000001"; + String zlock2 = "zlock#00000000-0000-0000-0000-bbbbbbbbbbbb#0000000001"; EasyMock.expect(zoo.exists(basePath)).andReturn(true); EasyMock.expect(zoo.getChildren(basePath)).andReturn(List.of(host1, host2)); - EasyMock.expect(zoo.getData(basePath + "/" + host1)) + EasyMock.expect(zoo.getZooKeeper()).andReturn(zk); + EasyMock.expect(zk.getChildren(basePath + "/" + host1, null)).andReturn(List.of(zlock1)); + EasyMock.expect(zk.getData(basePath + "/" + host1 + "/" + zlock1, false, null)) .andReturn((UUID.randomUUID().toString() + ",default").getBytes(UTF_8)); - EasyMock.expect(zoo.getData(basePath + "/" + host2)) + EasyMock.expect(zk.getChildren(basePath + "/" + host2, null)).andReturn(List.of(zlock2)); + EasyMock.expect(zk.getData(basePath + "/" + host2 + "/" + zlock2, false, null)) .andReturn((UUID.randomUUID().toString() + ",rg1").getBytes(UTF_8)); - EasyMock.expect(zoo.exists(basePath + "/" + host1)).andReturn(true); - EasyMock.expect(zoo.getChildren(basePath + "/" + host1)).andReturn(List.of(lock1)); AtomicBoolean deletedHost1 = new AtomicBoolean(false); AtomicBoolean deletedHost2 = new AtomicBoolean(false); @@ -179,15 +192,13 @@ public void testSserverDeleteAllNoGroupFilter() throws Exception { return null; }); - EasyMock.expect(zoo.exists(basePath + "/" + host2)).andReturn(true); - EasyMock.expect(zoo.getChildren(basePath + "/" + host2)).andReturn(List.of(lock2)); zoo.recursiveDelete(basePath + "/" + host2, NodeMissingPolicy.SKIP); EasyMock.expectLastCall().andStubAnswer(() -> { deletedHost2.set(true); return null; }); - EasyMock.replay(zoo); + EasyMock.replay(zoo, zk); ZooZap.Opts opts = new ZooZap.Opts(); ZooZap.removeScanServerGroupLocks(zoo, basePath, hp -> true, g -> true, opts); @@ -198,5 +209,7 @@ public void testSserverDeleteAllNoGroupFilter() throws Exception { assertTrue(deletedHost2.get(), "Expected scan server lock for host2 to be deleted when no group filter is set."); }); + + EasyMock.verify(zoo, zk); } } From 49a9d1c1ea5bca8ddcccd874e4631e623e58b3a5 Mon Sep 17 00:00:00 2001 From: Dave Marion Date: Fri, 23 Jan 2026 20:26:51 +0000 Subject: [PATCH 4/7] Implemented PR suggestions --- .../accumulo/core/fate/zookeeper/ServiceLock.java | 13 +++++++++---- .../java/org/apache/accumulo/server/util/Admin.java | 3 ++- .../org/apache/accumulo/server/util/ZooZap.java | 7 ++++--- 3 files changed, 15 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 d072ea520ed..3d2c1bcd86e 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 @@ -774,14 +774,19 @@ public static void deleteScanServerLocks(ZooReaderWriter zk, String zPath, ZooKeeper z = zk.getZooKeeper(); for (String server : servers) { if (hostPortPredicate.test(HostAndPort.fromString(server))) { - byte[] lockData = ServiceLock.getLockData(z, path(zPath + "/" + server)); + final String serverPath = zPath + "/" + server; + byte[] lockData = ServiceLock.getLockData(z, path(serverPath)); + if (lockData == null) { + messageOutput.accept("Skipping server " + server + " as it's lock content is empty."); + continue; + } String lockContent = new String(lockData, UTF_8); String[] parts = lockContent.split(","); if (parts.length == 2 && groupPredicate.test(parts[1])) { - messageOutput.accept("Deleting " + zPath + "/" + server + " from zookeeper"); + messageOutput.accept("Deleting " + serverPath + " from zookeeper"); if (!dryRun) { - LOG.debug("Deleting all locks at path {} due to lock deletion", zPath); - zk.recursiveDelete(zPath + "/" + server, NodeMissingPolicy.SKIP); + LOG.debug("Deleting all locks at path {} due to lock deletion", serverPath); + zk.recursiveDelete(serverPath, 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 fc27501de29..b081922e6ff 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 @@ -591,7 +591,8 @@ private static void stopServers(final ServerContext context, List server String tserversPath = Constants.ZROOT + "/" + iid + Constants.ZTSERVERS; ZooZap.removeLocks(zk, tserversPath, hostAndPort::contains, opts); String compactorsBasepath = Constants.ZROOT + "/" + iid + Constants.ZCOMPACTORS; - ZooZap.removeGroupedLocks(zk, compactorsBasepath, rg -> true, hostAndPort::contains, opts); + 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); diff --git a/server/base/src/main/java/org/apache/accumulo/server/util/ZooZap.java b/server/base/src/main/java/org/apache/accumulo/server/util/ZooZap.java index 7f635efba6d..7a0f1d7efd9 100644 --- a/server/base/src/main/java/org/apache/accumulo/server/util/ZooZap.java +++ b/server/base/src/main/java/org/apache/accumulo/server/util/ZooZap.java @@ -235,7 +235,8 @@ public void zap(SiteConfiguration siteConf, String... args) { if (opts.zapCompactors) { String compactorsBasepath = Constants.ZROOT + "/" + iid + Constants.ZCOMPACTORS; try { - removeGroupedLocks(zoo, compactorsBasepath, groupPredicate, hostPortPredicate, opts); + removeCompactorGroupedLocks(zoo, compactorsBasepath, groupPredicate, hostPortPredicate, + opts); } catch (KeeperException | InterruptedException e) { log.error("Error deleting compactors from zookeeper", e); } @@ -263,8 +264,8 @@ private static void zapDirectory(ZooReaderWriter zoo, String path, Opts opts) } } - static void removeGroupedLocks(ZooReaderWriter zoo, String path, Predicate groupPredicate, - Predicate hostPortPredicate, Opts opts) + static void removeCompactorGroupedLocks(ZooReaderWriter zoo, String path, + Predicate groupPredicate, Predicate hostPortPredicate, Opts opts) throws KeeperException, InterruptedException { if (zoo.exists(path)) { List groups = zoo.getChildren(path); From be8d7dea4bf99d36491ea5643d18d3af4d20614c Mon Sep 17 00:00:00 2001 From: Dave Marion Date: Fri, 23 Jan 2026 15:37:27 -0500 Subject: [PATCH 5/7] Update server/base/src/main/java/org/apache/accumulo/server/util/ZooZap.java Co-authored-by: Dom G. --- .../main/java/org/apache/accumulo/server/util/ZooZap.java | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/server/base/src/main/java/org/apache/accumulo/server/util/ZooZap.java b/server/base/src/main/java/org/apache/accumulo/server/util/ZooZap.java index 7a0f1d7efd9..6954ba11bbd 100644 --- a/server/base/src/main/java/org/apache/accumulo/server/util/ZooZap.java +++ b/server/base/src/main/java/org/apache/accumulo/server/util/ZooZap.java @@ -246,7 +246,11 @@ public void zap(SiteConfiguration siteConf, String... args) { if (opts.zapScanServers) { String sserversPath = Constants.ZROOT + "/" + iid + Constants.ZSSERVERS; try { - removeScanServerGroupLocks(zoo, sserversPath, hostPortPredicate, groupPredicate, opts); + if (opts.includeGroups == null) { + removeLocks(zoo, sserversPath, hostPortPredicate, opts); + } else { + removeScanServerGroupLocks(zoo, sserversPath, hostPortPredicate, groupPredicate, opts); + } } catch (KeeperException | InterruptedException e) { log.error("Error deleting scan server locks", e); } From 1ca9803d189e404bf06a6ac266d2dd090a6343c0 Mon Sep 17 00:00:00 2001 From: "Dom G." Date: Tue, 27 Jan 2026 08:00:23 -0500 Subject: [PATCH 6/7] use same short-circuit code path as prod code (#58) --- .../accumulo/server/util/AdminTest.java | 67 +++---------------- 1 file changed, 9 insertions(+), 58 deletions(-) diff --git a/server/base/src/test/java/org/apache/accumulo/server/util/AdminTest.java b/server/base/src/test/java/org/apache/accumulo/server/util/AdminTest.java index a86b8b16f8c..15645f87bff 100644 --- a/server/base/src/test/java/org/apache/accumulo/server/util/AdminTest.java +++ b/server/base/src/test/java/org/apache/accumulo/server/util/AdminTest.java @@ -19,15 +19,11 @@ package org.apache.accumulo.server.util; import static java.nio.charset.StandardCharsets.UTF_8; -import static org.junit.jupiter.api.Assertions.assertAll; import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.api.Assertions.assertFalse; -import static org.junit.jupiter.api.Assertions.assertTrue; import java.util.Collections; import java.util.List; import java.util.UUID; -import java.util.concurrent.atomic.AtomicBoolean; import org.apache.accumulo.core.Constants; import org.apache.accumulo.core.clientImpl.ClientContext; @@ -109,8 +105,8 @@ public void testCannotQualifySessionId() { @Test public void testSserverGroupFilterUsesLockData() throws Exception { - ZooReaderWriter zoo = EasyMock.createNiceMock(ZooReaderWriter.class); - ZooKeeper zk = EasyMock.createNiceMock(ZooKeeper.class); + ZooReaderWriter zoo = EasyMock.createMock(ZooReaderWriter.class); + ZooKeeper zk = EasyMock.createMock(ZooKeeper.class); String basePath = "/accumulo/iid/sservers"; String hostDefault = "host1:10000"; @@ -128,31 +124,13 @@ public void testSserverGroupFilterUsesLockData() throws Exception { EasyMock.expect(zk.getData(basePath + "/" + hostOther + "/" + zlock2, false, null)) .andReturn((UUID.randomUUID().toString() + ",rg1").getBytes(UTF_8)); - AtomicBoolean deletedDefault = new AtomicBoolean(false); - AtomicBoolean deletedOther = new AtomicBoolean(false); - zoo.recursiveDelete(basePath + "/" + hostDefault, NodeMissingPolicy.SKIP); - EasyMock.expectLastCall().andStubAnswer(() -> { - deletedDefault.set(true); - return null; - }); - - zoo.recursiveDelete(basePath + "/" + hostOther, NodeMissingPolicy.SKIP); - EasyMock.expectLastCall().andStubAnswer(() -> { - deletedOther.set(true); - return null; - }); + EasyMock.expectLastCall(); EasyMock.replay(zoo, zk); ZooZap.Opts opts = new ZooZap.Opts(); ZooZap.removeScanServerGroupLocks(zoo, basePath, hp -> true, "default"::equals, opts); - assertAll(() -> { - assertTrue(deletedDefault.get(), - "Expected scan server lock for group 'default' to be deleted, but it was not."); - assertFalse(deletedOther.get(), - "Expected scan server lock for group 'rg1' to be preserved, but it was deleted."); - }); EasyMock.verify(zoo, zk); @@ -161,55 +139,28 @@ public void testSserverGroupFilterUsesLockData() throws Exception { /** * SServer cleanup without group filter should delete all host nodes. */ - @SuppressWarnings("deprecation") @Test public void testSserverDeleteAllNoGroupFilter() throws Exception { - ZooReaderWriter zoo = EasyMock.createNiceMock(ZooReaderWriter.class); - ZooKeeper zk = EasyMock.createNiceMock(ZooKeeper.class); + ZooReaderWriter zoo = EasyMock.createMock(ZooReaderWriter.class); String basePath = "/accumulo/iid/sservers"; String host1 = "host1:10000"; String host2 = "host2:10001"; - String zlock1 = "zlock#00000000-0000-0000-0000-aaaaaaaaaaaa#0000000001"; - String zlock2 = "zlock#00000000-0000-0000-0000-bbbbbbbbbbbb#0000000001"; EasyMock.expect(zoo.exists(basePath)).andReturn(true); EasyMock.expect(zoo.getChildren(basePath)).andReturn(List.of(host1, host2)); - EasyMock.expect(zoo.getZooKeeper()).andReturn(zk); - EasyMock.expect(zk.getChildren(basePath + "/" + host1, null)).andReturn(List.of(zlock1)); - EasyMock.expect(zk.getData(basePath + "/" + host1 + "/" + zlock1, false, null)) - .andReturn((UUID.randomUUID().toString() + ",default").getBytes(UTF_8)); - EasyMock.expect(zk.getChildren(basePath + "/" + host2, null)).andReturn(List.of(zlock2)); - EasyMock.expect(zk.getData(basePath + "/" + host2 + "/" + zlock2, false, null)) - .andReturn((UUID.randomUUID().toString() + ",rg1").getBytes(UTF_8)); - - AtomicBoolean deletedHost1 = new AtomicBoolean(false); - AtomicBoolean deletedHost2 = new AtomicBoolean(false); zoo.recursiveDelete(basePath + "/" + host1, NodeMissingPolicy.SKIP); - EasyMock.expectLastCall().andStubAnswer(() -> { - deletedHost1.set(true); - return null; - }); + EasyMock.expectLastCall(); zoo.recursiveDelete(basePath + "/" + host2, NodeMissingPolicy.SKIP); - EasyMock.expectLastCall().andStubAnswer(() -> { - deletedHost2.set(true); - return null; - }); + EasyMock.expectLastCall(); - EasyMock.replay(zoo, zk); + EasyMock.replay(zoo); ZooZap.Opts opts = new ZooZap.Opts(); - ZooZap.removeScanServerGroupLocks(zoo, basePath, hp -> true, g -> true, opts); - - assertAll(() -> { - assertTrue(deletedHost1.get(), - "Expected scan server lock for host1 to be deleted when no group filter is set."); - assertTrue(deletedHost2.get(), - "Expected scan server lock for host2 to be deleted when no group filter is set."); - }); + ZooZap.removeLocks(zoo, basePath, hp -> true, opts); - EasyMock.verify(zoo, zk); + EasyMock.verify(zoo); } } From e56a53183f6920abcc0e8dce0ea4b9e390a08848 Mon Sep 17 00:00:00 2001 From: Dave Marion Date: Tue, 27 Jan 2026 13:16:46 +0000 Subject: [PATCH 7/7] Remove deprecated annotation --- .../org/apache/accumulo/core/fate/zookeeper/ServiceLock.java | 1 - 1 file changed, 1 deletion(-) 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 3d2c1bcd86e..989dd718c60 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 @@ -755,7 +755,6 @@ public static void deleteLocks(ZooReaderWriter zk, String zPath, } } - @Deprecated(since = "2.1.5") public static void deleteScanServerLocks(ZooReaderWriter zk, String zPath, Predicate hostPortPredicate, Predicate groupPredicate, Consumer messageOutput, Boolean dryRun) throws KeeperException, InterruptedException {