Repository: pinpoint-apm/pinpoint
Issue: Remove unnecessary agent status queries when drawing the server map
Labels: enhancement

Agent status check is necessary to add agent to nodes that do not have call statistics.

Agents that do have these statistics should be added to the server map node regardless of their current status, which makes checking for their status unnecessary.

Removing these unnecessary agent status queries should speed up server map creation time.


Issue comments:

Pull request review comments:

Patch that resolved the issue:
diff --git a/web/src/main/java/com/navercorp/pinpoint/web/applicationmap/ApplicationMapBuilder.java b/web/src/main/java/com/navercorp/pinpoint/web/applicationmap/ApplicationMapBuilder.java
index a40d58645ede..fac29651ea52 100644
--- a/web/src/main/java/com/navercorp/pinpoint/web/applicationmap/ApplicationMapBuilder.java
+++ b/web/src/main/java/com/navercorp/pinpoint/web/applicationmap/ApplicationMapBuilder.java
@@ -57,17 +57,17 @@ public ApplicationMap build(Application application, AgentInfoService agentInfoS
         
         Node node = new Node(application);
         Set<AgentInfo> agentInfos = agentInfoService.getAgentsByApplicationName(application.getName(), range.getTo());
-        for (Iterator<AgentInfo> iterator = agentInfos.iterator(); iterator.hasNext();) {
-            AgentInfo agentInfo = iterator.next();
-            if (!isAgentRunning(agentInfo)) {
-                iterator.remove();
+        Set<AgentInfo> runningAgents = new HashSet<>();
+        for (AgentInfo agentInfo : agentInfos) {
+            if (isAgentRunning(agentInfo)) {
+                runningAgents.add(agentInfo);
             }
         }
-        if (agentInfos.isEmpty()) {
+        if (runningAgents.isEmpty()) {
             return new ApplicationMap(range, nodeList, emptyLinkList);
         } else {
             ServerBuilder serverBuilder = new ServerBuilder();
-            serverBuilder.addAgentInfo(agentInfos);
+            serverBuilder.addAgentInfo(runningAgents);
             ServerInstanceList serverInstanceList = serverBuilder.build();
             node.setServerInstanceList(serverInstanceList);
             node.setNodeHistogram(new NodeHistogram(application, range));
@@ -100,7 +100,8 @@ public ApplicationMap build(LinkDataDuplexMap linkDataDuplexMap, final AgentInfo
         AgentInfoPopulator agentInfoPopulator = new AgentInfoPopulator() {
             @Override
             public void addAgentInfos(Node node) {
-                Set<AgentInfo> agentList = agentInfoService.getAgentsByApplicationName(node.getApplication().getName(), range.getTo());
+                long timestamp = range.getTo();
+                Set<AgentInfo> agentList = agentInfoService.getAgentsByApplicationNameWithoutStatus(node.getApplication().getName(), timestamp);
                 if (agentList.isEmpty()) {
                     logger.warn("agentInfo not found. applicationName:{}", node.getApplication());
                     // avoid NPE
@@ -109,7 +110,7 @@ public void addAgentInfos(Node node) {
                 }
                 logger.debug("add agentInfo. {}, {}", node.getApplication(), agentList);
                 ServerBuilder builder = new ServerBuilder();
-                agentList = filterAgentInfoByResponseData(agentList, node);
+                agentList = filterAgentInfoByResponseData(agentList, timestamp, node, agentInfoService);
                 builder.addAgentInfo(agentList);
                 ServerInstanceList serverInstanceList = builder.build();
 
@@ -447,7 +448,7 @@ private void appendServerInfo(Node node, LinkDataDuplexMap linkDataDuplexMap, Ag
      * Filters AgentInfo by whether they actually have response data.
      * For agents that do not have response data, check their status and include those that were alive.
      */
-    private Set<AgentInfo> filterAgentInfoByResponseData(Set<AgentInfo> agentList, Node node) {
+    private Set<AgentInfo> filterAgentInfoByResponseData(Set<AgentInfo> agentList, long timestamp, Node node, AgentInfoService agentInfoService) {
         Set<AgentInfo> filteredAgentInfo = new HashSet<>();
 
         NodeHistogram nodeHistogram = node.getNodeHistogram();
@@ -457,12 +458,15 @@ private Set<AgentInfo> filterAgentInfoByResponseData(Set<AgentInfo> agentList, N
             if (agentHistogramMap.containsKey(agentId)) {
                 filteredAgentInfo.add(agentInfo);
             } else {
+                AgentStatus status = agentInfoService.getAgentStatus(agentId, timestamp);
+                agentInfo.setStatus(status);
                 if (isAgentRunning(agentInfo)) {
                     filteredAgentInfo.add(agentInfo);
                 }
             }
         }
 
+
         return filteredAgentInfo;
     }
     
diff --git a/web/src/main/java/com/navercorp/pinpoint/web/dao/AgentLifeCycleDao.java b/web/src/main/java/com/navercorp/pinpoint/web/dao/AgentLifeCycleDao.java
index b0315761580f..efebaae3eb29 100644
--- a/web/src/main/java/com/navercorp/pinpoint/web/dao/AgentLifeCycleDao.java
+++ b/web/src/main/java/com/navercorp/pinpoint/web/dao/AgentLifeCycleDao.java
@@ -19,7 +19,7 @@
 import com.navercorp.pinpoint.web.vo.AgentInfo;
 import com.navercorp.pinpoint.web.vo.AgentStatus;
 
-import java.util.List;
+import java.util.Collection;
 
 /**
  * @author HyunGil Jeong
@@ -30,6 +30,6 @@ public interface AgentLifeCycleDao {
 
     void populateAgentStatus(AgentInfo agentInfo, long timestamp);
 
-    void populateAgentStatuses(List<AgentInfo> agentInfos, long timestamp);
+    void populateAgentStatuses(Collection<AgentInfo> agentInfos, long timestamp);
 
 }
diff --git a/web/src/main/java/com/navercorp/pinpoint/web/dao/hbase/HbaseAgentLifeCycleDao.java b/web/src/main/java/com/navercorp/pinpoint/web/dao/hbase/HbaseAgentLifeCycleDao.java
index 64f64a49b7ab..071caa285666 100644
--- a/web/src/main/java/com/navercorp/pinpoint/web/dao/hbase/HbaseAgentLifeCycleDao.java
+++ b/web/src/main/java/com/navercorp/pinpoint/web/dao/hbase/HbaseAgentLifeCycleDao.java
@@ -17,10 +17,10 @@
 package com.navercorp.pinpoint.web.dao.hbase;
 
 import java.util.ArrayList;
+import java.util.Collection;
 import java.util.List;
 
 import com.navercorp.pinpoint.common.util.AgentLifeCycleState;
-import com.navercorp.pinpoint.web.mapper.AgentLifeCycleMapper;
 import com.navercorp.pinpoint.web.vo.AgentInfo;
 import com.navercorp.pinpoint.web.vo.AgentStatus;
 import org.apache.commons.collections.CollectionUtils;
@@ -93,7 +93,7 @@ public void populateAgentStatus(AgentInfo agentInfo, long timestamp) {
     }
 
     @Override
-    public void populateAgentStatuses(List<AgentInfo> agentInfos, long timestamp) {
+    public void populateAgentStatuses(Collection<AgentInfo> agentInfos, long timestamp) {
         if (CollectionUtils.isEmpty(agentInfos)) {
             return;
         }
diff --git a/web/src/main/java/com/navercorp/pinpoint/web/service/AgentInfoService.java b/web/src/main/java/com/navercorp/pinpoint/web/service/AgentInfoService.java
index f6ee044ad004..ac107b5de669 100644
--- a/web/src/main/java/com/navercorp/pinpoint/web/service/AgentInfoService.java
+++ b/web/src/main/java/com/navercorp/pinpoint/web/service/AgentInfoService.java
@@ -41,7 +41,9 @@ public interface AgentInfoService {
 
     Set<AgentInfo> getAgentsByApplicationName(String applicationName, long timestamp);
 
-    Set<AgentInfo> getAgentsByApplicationName(String applicationName, long timestamp, long timeDiff);
+    Set<AgentInfo> getAgentsByApplicationNameWithoutStatus(String applicationName, long timestamp);
+
+    Set<AgentInfo> getRecentAgentsByApplicationName(String applicationName, long timestamp, long timeDiff);
 
     AgentInfo getAgentInfo(String agentId, long timestamp);
 
diff --git a/web/src/main/java/com/navercorp/pinpoint/web/service/AgentInfoServiceImpl.java b/web/src/main/java/com/navercorp/pinpoint/web/service/AgentInfoServiceImpl.java
index a1e787a58875..7c110b411878 100644
--- a/web/src/main/java/com/navercorp/pinpoint/web/service/AgentInfoServiceImpl.java
+++ b/web/src/main/java/com/navercorp/pinpoint/web/service/AgentInfoServiceImpl.java
@@ -166,29 +166,39 @@ private List<String> getApplicationNameList(List<Application> applications) {
 
     @Override
     public Set<AgentInfo> getAgentsByApplicationName(String applicationName, long timestamp) {
-        return this.getAgentsByApplicationName(applicationName, timestamp, timestamp);
+        Set<AgentInfo> agentInfos = this.getAgentsByApplicationNameWithoutStatus(applicationName, timestamp);
+        this.agentLifeCycleDao.populateAgentStatuses(agentInfos, timestamp);
+        return agentInfos;
     }
 
     @Override
-    public Set<AgentInfo> getAgentsByApplicationName(String applicationName, long timestamp, long timeDiff) {
+    public Set<AgentInfo> getAgentsByApplicationNameWithoutStatus(String applicationName, long timestamp) {
         if (applicationName == null) {
             throw new NullPointerException("applicationName must not be null");
         }
         if (timestamp < 0) {
             throw new IllegalArgumentException("timestamp must not be less than 0");
         }
+
+        List<String> agentIds = this.applicationIndexDao.selectAgentIds(applicationName);
+        List<AgentInfo> agentInfos = this.agentInfoDao.getAgentInfos(agentIds, timestamp);
+        CollectionUtils.filter(agentInfos, PredicateUtils.notNullPredicate());
+        if (CollectionUtils.isEmpty(agentInfos)) {
+            return Collections.emptySet();
+        }
+        return new HashSet<>(agentInfos);
+    }
+
+    @Override
+    public Set<AgentInfo> getRecentAgentsByApplicationName(String applicationName, long timestamp, long timeDiff) {
         if (timeDiff > timestamp) {
             throw new IllegalArgumentException("timeDiff must not be greater than timestamp");
         }
+
+        Set<AgentInfo> unfilteredAgentInfos = this.getAgentsByApplicationName(applicationName, timestamp);
+
         final long eventTimestampFloor = timestamp - timeDiff;
 
-        List<String> agentIds = this.applicationIndexDao.selectAgentIds(applicationName);
-        List<AgentInfo> unfilteredAgentInfos = this.agentInfoDao.getAgentInfos(agentIds, timestamp);
-        if (unfilteredAgentInfos == null || unfilteredAgentInfos.isEmpty()) {
-            return Collections.emptySet();
-        }
-        CollectionUtils.filter(unfilteredAgentInfos, PredicateUtils.notNullPredicate());
-        this.agentLifeCycleDao.populateAgentStatuses(unfilteredAgentInfos, timestamp);
         Set<AgentInfo> filteredAgentInfos = new HashSet<>();
         for (AgentInfo agentInfo : unfilteredAgentInfos) {
             AgentStatus agentStatus = agentInfo.getStatus();
diff --git a/web/src/main/java/com/navercorp/pinpoint/web/service/AgentServiceImpl.java b/web/src/main/java/com/navercorp/pinpoint/web/service/AgentServiceImpl.java
index 3bb34fa88fa7..00053762d74e 100644
--- a/web/src/main/java/com/navercorp/pinpoint/web/service/AgentServiceImpl.java
+++ b/web/src/main/java/com/navercorp/pinpoint/web/service/AgentServiceImpl.java
@@ -149,7 +149,7 @@ public List<AgentInfo> getRecentAgentInfoList(String applicationName, long timeD
 
         long currentTime = System.currentTimeMillis();
 
-        Set<AgentInfo> agentInfos = agentInfoService.getAgentsByApplicationName(applicationName, currentTime, timeDiff);
+        Set<AgentInfo> agentInfos = agentInfoService.getRecentAgentsByApplicationName(applicationName, currentTime, timeDiff);
         for (AgentInfo agentInfo : agentInfos) {
             ListUtils.addIfValueNotNull(agentInfoList, agentInfo);
         }


Test patch: