[#11050] Replace StopFlag with CompletableFuture.cancel

pinpoint-apm · Jun 5, 2024 · 79bdb7f · 79bdb7f
1 parent 044c8be
commit 79bdb7f
Show file tree

Hide file tree

Showing 3 changed files with 38 additions and 40 deletions.
diff --git a/.../com/navercorp/pinpoint/web/applicationmap/appender/server/DefaultServerInfoAppender.java b/.../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<Node> nodes = source.getNodeList();
         if (CollectionUtils.isEmpty(nodes)) {
             return;
         }
 
         final CompletableFuture<ServerGroupList>[] 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<ServerGroupList> 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<ServerGroupList>[] getServerGroupListFutures(Range range, Collection<Node> nodes, LinkDataDuplexMap linkDataDuplexMap) {
         List<CompletableFuture<ServerGroupList>> 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<ServerGroupList> serverGroupListFuture = getServerGroupListFuture(range, node, linkDataDuplexMap);
             serverGroupListFutures.add(serverGroupListFuture);
         }
-
         return serverGroupListFutures.toArray(new CompletableFuture[0]);
     }
 
-    private CompletableFuture<ServerGroupList> getServerGroupListFuture(Range range, Node node, LinkDataDuplexMap linkDataDuplexMap) {
-        CompletableFuture<ServerGroupList> 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<ServerGroupList> getServerGroupListFuture0(Node node, Range range, LinkDataDuplexMap linkDataDuplexMap) {
+    private CompletableFuture<ServerGroupList> 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
@@ -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
@@ -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();