From f73b6d0084b8141477b2a2ab86a599e2670506eb Mon Sep 17 00:00:00 2001 From: Keith Turner Date: Wed, 8 Apr 2026 23:43:12 +0000 Subject: [PATCH 1/3] Modified InstanceOperations.getServer(MANAGER) to return all This used to only return the primary manager. --- .../clientImpl/InstanceOperationsImpl.java | 14 ++------------ .../core/rpc/clients/ManagerClient.java | 18 +++++++++++++----- 2 files changed, 15 insertions(+), 17 deletions(-) diff --git a/core/src/main/java/org/apache/accumulo/core/clientImpl/InstanceOperationsImpl.java b/core/src/main/java/org/apache/accumulo/core/clientImpl/InstanceOperationsImpl.java index af9c4e3ca4e..5278da565f2 100644 --- a/core/src/main/java/org/apache/accumulo/core/clientImpl/InstanceOperationsImpl.java +++ b/core/src/main/java/org/apache/accumulo/core/clientImpl/InstanceOperationsImpl.java @@ -561,18 +561,8 @@ private Set getServers(ServerId.Type type, .forEach(c -> results.add(createServerId(type, c))); break; case MANAGER: - ServiceLockPath m = context.getServerPaths().getManager(true); - if (m != null) { - Optional sld = context.getZooCache().getLockData(m); - String location = null; - if (sld.isPresent()) { - location = sld.orElseThrow().getAddressString(ThriftService.MANAGER); - if (location != null && addressSelector.getPredicate().test(location)) { - HostAndPort hp = HostAndPort.fromString(location); - results.add(new ServerId(type, ResourceGroupId.DEFAULT, hp.getHost(), hp.getPort())); - } - } - } + context.getServerPaths().getAssistantManagers(AddressSelector.all(), true) + .forEach(s -> results.add(createServerId(type, s))); break; case MONITOR: ServiceLockPath mon = context.getServerPaths().getMonitor(true); diff --git a/core/src/main/java/org/apache/accumulo/core/rpc/clients/ManagerClient.java b/core/src/main/java/org/apache/accumulo/core/rpc/clients/ManagerClient.java index 5127807e0b2..b044178fa44 100644 --- a/core/src/main/java/org/apache/accumulo/core/rpc/clients/ManagerClient.java +++ b/core/src/main/java/org/apache/accumulo/core/rpc/clients/ManagerClient.java @@ -21,10 +21,11 @@ import static com.google.common.base.Preconditions.checkArgument; import java.net.UnknownHostException; -import java.util.Set; +import java.util.Optional; -import org.apache.accumulo.core.client.admin.servers.ServerId; import org.apache.accumulo.core.clientImpl.ClientContext; +import org.apache.accumulo.core.lock.ServiceLockData; +import org.apache.accumulo.core.lock.ServiceLockPaths; import org.apache.accumulo.core.rpc.ThriftUtil; import org.apache.thrift.TServiceClient; import org.apache.thrift.transport.TTransportException; @@ -37,14 +38,21 @@ public interface ManagerClient { default C getManagerConnection(Logger log, ThriftClientTypes type, ClientContext context) { checkArgument(context != null, "context is null"); - Set managers = context.instanceOperations().getServers(ServerId.Type.MANAGER); + // obtain the primary manager location + String managerLocation = null; + ServiceLockPaths.ServiceLockPath m = context.getServerPaths().getManager(true); + if (m != null) { + Optional sld = context.getZooCache().getLockData(m); + if (sld.isPresent()) { + managerLocation = sld.orElseThrow().getAddressString(ServiceLockData.ThriftService.MANAGER); + } + } - if (managers == null || managers.isEmpty()) { + if (managerLocation == null) { log.debug("No managers..."); return null; } - final String managerLocation = managers.iterator().next().toHostPortString(); if (managerLocation.equals("0.0.0.0:0")) { // The Manager creates the lock with an initial address of 0.0.0.0:0, then // later updates the lock contents with the actual address after everything From 3e33d822d039c1e023e03cb6709e52b32c3481e0 Mon Sep 17 00:00:00 2001 From: Keith Turner Date: Thu, 9 Apr 2026 15:15:34 +0000 Subject: [PATCH 2/3] fix problems found reviewing usage of ServerType.Manager --- .../clientImpl/InstanceOperationsImpl.java | 4 +-- .../org/apache/accumulo/server/util/Info.java | 6 +--- .../checkCommand/SystemConfigCheckRunner.java | 4 +-- .../accumulo/test/InstanceOperationsIT.java | 28 +++++++++++++++++-- 4 files changed, 30 insertions(+), 12 deletions(-) diff --git a/core/src/main/java/org/apache/accumulo/core/clientImpl/InstanceOperationsImpl.java b/core/src/main/java/org/apache/accumulo/core/clientImpl/InstanceOperationsImpl.java index 5278da565f2..35f3dfcc7ac 100644 --- a/core/src/main/java/org/apache/accumulo/core/clientImpl/InstanceOperationsImpl.java +++ b/core/src/main/java/org/apache/accumulo/core/clientImpl/InstanceOperationsImpl.java @@ -233,7 +233,7 @@ public List getManagerLocations() { if (managers == null || managers.isEmpty()) { return List.of(); } else { - return List.of(managers.iterator().next().toHostPortString()); + return managers.stream().map(ServerId::toHostPortString).toList(); } } @@ -561,7 +561,7 @@ private Set getServers(ServerId.Type type, .forEach(c -> results.add(createServerId(type, c))); break; case MANAGER: - context.getServerPaths().getAssistantManagers(AddressSelector.all(), true) + context.getServerPaths().getAssistantManagers(addressSelector, true) .forEach(s -> results.add(createServerId(type, s))); break; case MONITOR: diff --git a/server/base/src/main/java/org/apache/accumulo/server/util/Info.java b/server/base/src/main/java/org/apache/accumulo/server/util/Info.java index 6120cf633ea..dd8cebe4678 100644 --- a/server/base/src/main/java/org/apache/accumulo/server/util/Info.java +++ b/server/base/src/main/java/org/apache/accumulo/server/util/Info.java @@ -57,13 +57,9 @@ public String description() { public void execute(JCommander cl, ServerOpts options) throws Exception { ServerContext context = getServerContext(); Set managers = context.instanceOperations().getServers(ServerId.Type.MANAGER); - String manager = null; - if (managers != null && !managers.isEmpty()) { - manager = managers.iterator().next().getHost(); - } System.out.println("monitor: " + MonitorUtil.getLocation(context)); - System.out.println("managers: " + manager); + System.out.println("managers: " + managers); System.out.println("zookeepers: " + context.getZooKeepers()); } } diff --git a/server/base/src/main/java/org/apache/accumulo/server/util/checkCommand/SystemConfigCheckRunner.java b/server/base/src/main/java/org/apache/accumulo/server/util/checkCommand/SystemConfigCheckRunner.java index 73a9ac9c463..93a94184994 100644 --- a/server/base/src/main/java/org/apache/accumulo/server/util/checkCommand/SystemConfigCheckRunner.java +++ b/server/base/src/main/java/org/apache/accumulo/server/util/checkCommand/SystemConfigCheckRunner.java @@ -76,8 +76,6 @@ private static boolean checkZKLocks(ServerContext context) throws Exception { var servers = context.instanceOperations().getServers(serverType); switch (serverType) { - case MANAGER: - // essential process case GARBAGE_COLLECTOR: // essential process if (servers.size() != 1) { @@ -101,6 +99,8 @@ private static boolean checkZKLocks(ServerContext context) throws Exception { log.trace("Verified ZooKeeper lock for {}", servers); } break; + case MANAGER: + // essential process(es) case TABLET_SERVER: // essential process(es) case COMPACTOR: diff --git a/test/src/main/java/org/apache/accumulo/test/InstanceOperationsIT.java b/test/src/main/java/org/apache/accumulo/test/InstanceOperationsIT.java index 0b4dadc9913..8ed46150c20 100644 --- a/test/src/main/java/org/apache/accumulo/test/InstanceOperationsIT.java +++ b/test/src/main/java/org/apache/accumulo/test/InstanceOperationsIT.java @@ -31,12 +31,13 @@ import org.apache.accumulo.core.client.Accumulo; import org.apache.accumulo.core.client.AccumuloClient; import org.apache.accumulo.core.client.AccumuloException; -import org.apache.accumulo.core.client.AccumuloSecurityException; import org.apache.accumulo.core.client.admin.InstanceOperations; import org.apache.accumulo.core.client.admin.servers.ServerId; import org.apache.accumulo.core.conf.Property; import org.apache.accumulo.core.data.ResourceGroupId; import org.apache.accumulo.harness.AccumuloClusterHarness; +import org.apache.accumulo.minicluster.ServerType; +import org.apache.accumulo.miniclusterImpl.MiniAccumuloClusterImpl; import org.apache.accumulo.miniclusterImpl.MiniAccumuloConfigImpl; import org.apache.accumulo.test.util.Wait; import org.apache.hadoop.conf.Configuration; @@ -60,7 +61,7 @@ public void configureMiniCluster(MiniAccumuloConfigImpl cfg, Configuration hadoo * Verify that we get the same servers from getServers() and getServer() */ @Test - public void testGetServer() { + public void testGetServer() throws Exception { try (AccumuloClient client = Accumulo.newClient().from(getClientProps()).build()) { final InstanceOperations iops = client.instanceOperations(); @@ -108,6 +109,20 @@ public void testGetServer() { assertEquals(expectedServerId, actualServerId); }); + var cluster = (MiniAccumuloClusterImpl) getCluster(); + cluster.getConfig().getClusterServerConfiguration().setNumManagers(3); + cluster.getClusterControl().start(ServerType.MANAGER); + Wait.waitFor(() -> iops.getServers(ServerId.Type.MANAGER).size() == 3); + final Set managers3 = iops.getServers(ServerId.Type.MANAGER); + assertEquals(3, managers3.size()); // Assuming there is only one manager + managers3.forEach(expectedServerId -> { + ServerId actualServerId = + iops.getServer(expectedServerId.getType(), expectedServerId.getResourceGroup(), + expectedServerId.getHost(), expectedServerId.getPort()); + assertNotNull(actualServerId, "Expected to find manager " + expectedServerId); + assertEquals(expectedServerId, actualServerId); + }); + // verify GC final Set gcs = iops.getServers(ServerId.Type.GARBAGE_COLLECTOR); assertEquals(1, gcs.size()); // Assuming there is only one garbage collector @@ -128,7 +143,7 @@ public void testGetServer() { @SuppressWarnings("deprecation") @Test - public void testGetServers() throws AccumuloException, AccumuloSecurityException { + public void testGetServers() throws Exception { try (AccumuloClient client = Accumulo.newClient().from(getClientProps()).build()) { InstanceOperations iops = client.instanceOperations(); @@ -147,6 +162,13 @@ public void testGetServers() throws AccumuloException, AccumuloSecurityException assertEquals(1, iops.getServers(ServerId.Type.MANAGER).size()); assertEquals(1, iops.getManagerLocations().size()); validateAddresses(iops.getManagerLocations(), iops.getServers(ServerId.Type.MANAGER)); + var cluster = (MiniAccumuloClusterImpl) getCluster(); + cluster.getConfig().getClusterServerConfiguration().setNumManagers(3); + cluster.getClusterControl().start(ServerType.MANAGER); + Wait.waitFor(() -> iops.getServers(ServerId.Type.MANAGER).size() == 3); + assertEquals(3, iops.getServers(ServerId.Type.MANAGER).size()); + assertEquals(3, iops.getManagerLocations().size()); + validateAddresses(iops.getManagerLocations(), iops.getServers(ServerId.Type.MANAGER)); assertEquals(1, iops.getServers(ServerId.Type.GARBAGE_COLLECTOR).size()); From 8054ed11114e1ae45268fc7f188deadbbcfe7a99 Mon Sep 17 00:00:00 2001 From: Keith Turner Date: Thu, 9 Apr 2026 15:21:49 +0000 Subject: [PATCH 3/3] fix comment --- .../java/org/apache/accumulo/test/InstanceOperationsIT.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/src/main/java/org/apache/accumulo/test/InstanceOperationsIT.java b/test/src/main/java/org/apache/accumulo/test/InstanceOperationsIT.java index 8ed46150c20..249244d65e2 100644 --- a/test/src/main/java/org/apache/accumulo/test/InstanceOperationsIT.java +++ b/test/src/main/java/org/apache/accumulo/test/InstanceOperationsIT.java @@ -114,7 +114,7 @@ public void testGetServer() throws Exception { cluster.getClusterControl().start(ServerType.MANAGER); Wait.waitFor(() -> iops.getServers(ServerId.Type.MANAGER).size() == 3); final Set managers3 = iops.getServers(ServerId.Type.MANAGER); - assertEquals(3, managers3.size()); // Assuming there is only one manager + assertEquals(3, managers3.size()); managers3.forEach(expectedServerId -> { ServerId actualServerId = iops.getServer(expectedServerId.getType(), expectedServerId.getResourceGroup(),