diff --git a/presto-main/src/main/java/com/facebook/presto/metadata/DiscoveryNodeManager.java b/presto-main/src/main/java/com/facebook/presto/metadata/DiscoveryNodeManager.java index 99d9a8d92dfd3..9797f3f4fdf2e 100644 --- a/presto-main/src/main/java/com/facebook/presto/metadata/DiscoveryNodeManager.java +++ b/presto-main/src/main/java/com/facebook/presto/metadata/DiscoveryNodeManager.java @@ -643,20 +643,16 @@ private static boolean isCoordinatorSidecar(ServiceDescriptor service) * Resource Manager -> All Nodes * Catalog Server -> All Nodes * Worker -> Resource Managers or Catalog Servers + * Sidecar -> Resource Managers or Catalog Servers * * @return Predicate to filter Service Descriptor for Nodes */ private Predicate filterRelevantNodes() { - if (currentNode.isCoordinator() || currentNode.isResourceManager() || currentNode.isCatalogServer() || currentNode.isCoordinatorSidecar()) { - // Allowing coordinator node in the list of services, even if it's not allowed by nodeStatusService with currentNode check - return service -> - !nodeStatusService.isPresent() - || nodeStatusService.get().isAllowed(service.getLocation()) - || isCatalogServer(service) - || isCoordinatorSidecar(service); + if (currentNode.isCoordinator() || currentNode.isResourceManager() || currentNode.isCatalogServer()) { + return service -> !nodeStatusService.isPresent() || nodeStatusService.get().isAllowed(service.getLocation()); } - return service -> isResourceManager(service) || isCatalogServer(service) || isCoordinatorSidecar(service); + return service -> isResourceManager(service) || isCatalogServer(service); } } diff --git a/presto-main/src/test/java/com/facebook/presto/metadata/TestDiscoveryNodeManager.java b/presto-main/src/test/java/com/facebook/presto/metadata/TestDiscoveryNodeManager.java index d7cc63f94bb09..84c9b438e0e88 100644 --- a/presto-main/src/test/java/com/facebook/presto/metadata/TestDiscoveryNodeManager.java +++ b/presto-main/src/test/java/com/facebook/presto/metadata/TestDiscoveryNodeManager.java @@ -169,7 +169,7 @@ public void testGetAllNodesForWorkerNode() AllNodes allNodes = manager.getAllNodes(); Set activeNodes = allNodes.getActiveNodes(); - assertEqualsIgnoreOrder(activeNodes, ImmutableSet.of(resourceManager, catalogServer, coordinatorSidecar)); + assertEqualsIgnoreOrder(activeNodes, ImmutableSet.of(resourceManager, catalogServer)); for (InternalNode actual : activeNodes) { for (InternalNode expected : this.activeNodes) { @@ -180,7 +180,7 @@ public void testGetAllNodesForWorkerNode() assertEqualsIgnoreOrder(activeNodes, manager.getNodes(ACTIVE)); Set inactiveNodes = allNodes.getInactiveNodes(); - assertEqualsIgnoreOrder(inactiveNodes, ImmutableSet.of(inActiveResourceManager, inActiveCatalogServer, inActiveCoordinatorSidecar)); + assertEqualsIgnoreOrder(inactiveNodes, ImmutableSet.of(inActiveResourceManager, inActiveCatalogServer)); for (InternalNode actual : inactiveNodes) { for (InternalNode expected : this.inactiveNodes) { @@ -271,7 +271,7 @@ public void testGetAllNodesForCoordinatorSidecar() AllNodes allNodes = manager.getAllNodes(); Set activeNodes = allNodes.getActiveNodes(); - assertEqualsIgnoreOrder(activeNodes, this.activeNodes); + assertEqualsIgnoreOrder(activeNodes, ImmutableSet.of(resourceManager, catalogServer)); for (InternalNode actual : activeNodes) { for (InternalNode expected : this.activeNodes) { @@ -282,7 +282,7 @@ public void testGetAllNodesForCoordinatorSidecar() assertEqualsIgnoreOrder(activeNodes, manager.getNodes(ACTIVE)); Set inactiveNodes = allNodes.getInactiveNodes(); - assertEqualsIgnoreOrder(inactiveNodes, this.inactiveNodes); + assertEqualsIgnoreOrder(inactiveNodes, ImmutableSet.of(inActiveResourceManager, inActiveCatalogServer)); for (InternalNode actual : inactiveNodes) { for (InternalNode expected : this.inactiveNodes) { @@ -298,11 +298,15 @@ public void testGetAllNodesForCoordinatorSidecar() } @Test - public void testGetCurrentNode() + public void testNodesVisibleToWorkerNode() { DiscoveryNodeManager manager = new DiscoveryNodeManager(selector, workerNodeInfo, new NoOpFailureDetector(), Optional.empty(), expectedVersion, testHttpClient, new TestingDriftClient<>(), internalCommunicationConfig); try { assertEquals(manager.getCurrentNode(), workerNode1); + assertEquals(manager.getCatalogServers(), ImmutableSet.of(catalogServer)); + assertEquals(manager.getResourceManagers(), ImmutableSet.of(resourceManager)); + assertEquals(manager.getCoordinatorSidecars(), ImmutableSet.of()); + assertEquals(manager.getCoordinators(), ImmutableSet.of()); } finally { manager.stop(); @@ -346,11 +350,14 @@ public void testGetCatalogServers() } @Test - public void testGetCoordinatorSidecar() + public void testNodesVisibleToCoordinatorSidecar() { DiscoveryNodeManager manager = new DiscoveryNodeManager(selector, coordinatorSidecarNodeInfo, new NoOpFailureDetector(), Optional.of(host -> false), expectedVersion, testHttpClient, new TestingDriftClient<>(), internalCommunicationConfig); try { - assertEquals(manager.getCoordinatorSidecars(), ImmutableSet.of(coordinatorSidecar)); + assertEquals(manager.getCatalogServers(), ImmutableSet.of(catalogServer)); + assertEquals(manager.getResourceManagers(), ImmutableSet.of(resourceManager)); + assertEquals(manager.getCoordinatorSidecars(), ImmutableSet.of()); + assertEquals(manager.getCoordinators(), ImmutableSet.of()); } finally { manager.stop();