From 79bdb7f5fd0a59a9207ef164b75a6d2ea28e537d Mon Sep 17 00:00:00 2001 From: emeroad Date: Wed, 5 Jun 2024 15:19:13 +0900 Subject: [PATCH] [#11050] Replace StopFlag with CompletableFuture.cancel --- .../server/DefaultServerInfoAppender.java | 68 ++++++++++--------- .../web/applicationmap/nodes/Node.java | 4 +- .../ApplicationMapBuilderTest.java | 6 -- 3 files changed, 38 insertions(+), 40 deletions(-) diff --git a/web/src/main/java/com/navercorp/pinpoint/web/applicationmap/appender/server/DefaultServerInfoAppender.java b/web/src/main/java/com/navercorp/pinpoint/web/applicationmap/appender/server/DefaultServerInfoAppender.java index 5edf7c0482a1..f74b18e4836c 100644 --- a/web/src/main/java/com/navercorp/pinpoint/web/applicationmap/appender/server/DefaultServerInfoAppender.java +++ b/web/src/main/java/com/navercorp/pinpoint/web/applicationmap/appender/server/DefaultServerInfoAppender.java @@ -55,29 +55,45 @@ public DefaultServerInfoAppender(ServerGroupListFactory serverGroupListFactory, } @Override - public void appendServerInfo(final Range range, final NodeList source, final LinkDataDuplexMap linkDataDuplexMap, final long timeoutMillis) { + public void appendServerInfo(final Range range, final NodeList source, final LinkDataDuplexMap linkDataDuplexMap, long timeoutMillis) { if (source == null) { return; } + if (-1 == timeoutMillis) { + timeoutMillis = Long.MAX_VALUE; + } + Collection nodes = source.getNodeList(); if (CollectionUtils.isEmpty(nodes)) { return; } final CompletableFuture[] futures = getServerGroupListFutures(range, nodes, linkDataDuplexMap); - if (-1 == timeoutMillis) { - // Returns the result value when complete - CompletableFuture.allOf(futures).join(); - } else { - try { - CompletableFuture.allOf(futures).get(timeoutMillis, TimeUnit.MILLISECONDS); - } catch (Throwable e) { - CompletableFuture.allOf(futures).cancel(false); - String cause = "an error occurred while adding server info"; - if (e instanceof TimeoutException) { - cause += " build timed out. timeout=" + timeoutMillis + "ms"; - } - throw new RuntimeException(cause, e); + + try { + CompletableFuture.allOf(futures).get(timeoutMillis, TimeUnit.MILLISECONDS); + } catch (Throwable e) { + CompletableFuture.allOf(futures).cancel(false); + String cause = "an error occurred while adding server info"; + if (e instanceof TimeoutException) { + cause += " build timed out. timeout=" + timeoutMillis + "ms"; + } + throw new RuntimeException(cause, e); + } + + int index = 0; + for (Node node : nodes) { + if (skipServerInfo(node)) { + continue; + } + final CompletableFuture future = futures[index++]; + if (future.isCompletedExceptionally()) { + logger.warn("Failed to get server info for node {}", node); + node.setServerGroupList(serverGroupListFactory.createEmptyNodeInstanceList()); + } else { + ServerGroupList serverGroupList = future.getNow(null); + logger.trace("ServerGroupList: {}", serverGroupList); + node.setServerGroupList(serverGroupList); } } } @@ -86,33 +102,21 @@ public void appendServerInfo(final Range range, final NodeList source, final Lin private CompletableFuture[] getServerGroupListFutures(Range range, Collection nodes, LinkDataDuplexMap linkDataDuplexMap) { List> serverGroupListFutures = new ArrayList<>(); for (Node node : nodes) { - if (node.getServiceType().isUnknown()) { - // we do not know the server info for unknown nodes + if (skipServerInfo(node)) { continue; } CompletableFuture serverGroupListFuture = getServerGroupListFuture(range, node, linkDataDuplexMap); serverGroupListFutures.add(serverGroupListFuture); } - return serverGroupListFutures.toArray(new CompletableFuture[0]); } - private CompletableFuture getServerGroupListFuture(Range range, Node node, LinkDataDuplexMap linkDataDuplexMap) { - CompletableFuture serverGroupListFuture = getServerGroupListFuture0(node, range, linkDataDuplexMap); - serverGroupListFuture.whenComplete((serverGroupList, throwable) -> { - if (throwable != null) { - // error - logger.warn("Failed to get server info for node {}", node, throwable); - node.setServerGroupList(serverGroupListFactory.createEmptyNodeInstanceList()); - } else { - logger.trace("ServerGroupList: {}", serverGroupList); - node.setServerGroupList(serverGroupList); - } - }); - return serverGroupListFuture; -} + private boolean skipServerInfo(Node node) { + // we do not know the server info for unknown nodes + return node.getServiceType().isUnknown(); + } - private CompletableFuture getServerGroupListFuture0(Node node, Range range, LinkDataDuplexMap linkDataDuplexMap) { + private CompletableFuture getServerGroupListFuture(Range range, Node node, LinkDataDuplexMap linkDataDuplexMap) { final Application application = node.getApplication(); final ServiceType nodeServiceType = application.getServiceType(); if (nodeServiceType.isWas()) { diff --git a/web/src/main/java/com/navercorp/pinpoint/web/applicationmap/nodes/Node.java b/web/src/main/java/com/navercorp/pinpoint/web/applicationmap/nodes/Node.java index 9d84a6a34086..f97514084f2e 100644 --- a/web/src/main/java/com/navercorp/pinpoint/web/applicationmap/nodes/Node.java +++ b/web/src/main/java/com/navercorp/pinpoint/web/applicationmap/nodes/Node.java @@ -39,9 +39,9 @@ public class Node { private final Application application; // avoid NPE - private volatile ServerGroupList serverGroupList = ServerGroupList.empty(); + private ServerGroupList serverGroupList = ServerGroupList.empty(); - private volatile NodeHistogram nodeHistogram; + private NodeHistogram nodeHistogram; private boolean authorized = true; private TimeHistogramFormat timeHistogramFormat = TimeHistogramFormat.V1; diff --git a/web/src/test/java/com/navercorp/pinpoint/web/applicationmap/ApplicationMapBuilderTest.java b/web/src/test/java/com/navercorp/pinpoint/web/applicationmap/ApplicationMapBuilderTest.java index 07c7f06a7d29..3fee26fce7a1 100644 --- a/web/src/test/java/com/navercorp/pinpoint/web/applicationmap/ApplicationMapBuilderTest.java +++ b/web/src/test/java/com/navercorp/pinpoint/web/applicationmap/ApplicationMapBuilderTest.java @@ -187,11 +187,9 @@ public void testNoCallData() { .includeServerInfo(serverGroupListFactory) .build(application, buildTimeoutMillis); - assertThat(applicationMap.getNodes()).hasSize(1); assertThat(applicationMap.getNodes()).hasSize(1); assertThat(applicationMap_parallelAppenders.getNodes()).hasSize(1); assertThat(applicationMap.getLinks()).isEmpty(); - assertThat(applicationMap.getLinks()).isEmpty(); assertThat(applicationMap_parallelAppenders.getLinks()).isEmpty(); ApplicationMapVerifier verifier = new ApplicationMapVerifier(applicationMap); @@ -218,11 +216,9 @@ public void testEmptyCallData() { .includeServerInfo(serverGroupListFactory) .build(linkDataDuplexMap, buildTimeoutMillis); - assertThat(applicationMap.getNodes()).isEmpty(); assertThat(applicationMap.getNodes()).isEmpty(); assertThat(applicationMap_parallelAppenders.getNodes()).isEmpty(); - assertThat(applicationMap.getLinks()).isEmpty(); assertThat(applicationMap.getLinks()).isEmpty(); assertThat(applicationMap_parallelAppenders.getLinks()).isEmpty(); @@ -250,11 +246,9 @@ public void testEmptyCallDataSimplfied() { .includeServerInfo(serverGroupListFactory) .build(linkDataDuplexMap, buildTimeoutMillis); - assertThat(applicationMap.getNodes()).isEmpty(); assertThat(applicationMap.getNodes()).isEmpty(); assertThat(applicationMap_parallelAppenders.getNodes()).isEmpty(); - assertThat(applicationMap.getLinks()).isEmpty(); assertThat(applicationMap.getLinks()).isEmpty(); assertThat(applicationMap_parallelAppenders.getLinks()).isEmpty();