[#noissue] Refactor ColumnGetCount

hungbv1 · Nov 2, 2021 · 01b5916 · 01b5916
1 parent cdac15a
commit 01b5916
Show file tree

Hide file tree

Showing 8 changed files with 69 additions and 45 deletions.
diff --git a/commons-hbase/src/main/java/com/navercorp/pinpoint/common/hbase/bo/ColumnGetCount.java b/commons-hbase/src/main/java/com/navercorp/pinpoint/common/hbase/bo/ColumnGetCount.java
@@ -26,9 +26,8 @@ public class ColumnGetCount {
     public static final ColumnGetCount UNLIMITED_COLUMN_GET_COUNT = new UnlimitedColumnGetCount();
 
     private final int limit;
-    private int resultSize;
 
-    public ColumnGetCount(int limit) {
+    ColumnGetCount(int limit) {
         Assert.isTrue(limit > 0, "limit must be 'limit >= 0'");
         this.limit = limit;
     }
@@ -37,13 +36,10 @@ public int getLimit() {
         return limit;
     }
 
-    public boolean isreachedLimit() {
+    public boolean isReachedLimit(int resultSize) {
         return resultSize >= limit;
     }
 
-    public void setResultSize(int resultSize) {
-        this.resultSize = resultSize;
-    }
 
     @Override
     public boolean equals(Object o) {
@@ -67,15 +63,10 @@ private UnlimitedColumnGetCount() {
         }
 
         @Override
-        public boolean isreachedLimit() {
+        public boolean isReachedLimit(int resultSize) {
             return false;
         }
 
-        @Override
-        public void setResultSize(int resultSize) {
-            // skip
-        }
-
     }
 
 }
diff --git a/...s-hbase/src/main/java/com/navercorp/pinpoint/common/hbase/rowmapper/ResultSizeMapper.java b/...s-hbase/src/main/java/com/navercorp/pinpoint/common/hbase/rowmapper/ResultSizeMapper.java
@@ -0,0 +1,17 @@
+package com.navercorp.pinpoint.common.hbase.rowmapper;
+
+import com.navercorp.pinpoint.common.hbase.RowMapper;
+import org.apache.hadoop.hbase.client.Result;
+
+public class ResultSizeMapper<T> implements RowMapper<T> {
+    private int resultSize;
+    @Override
+    public T mapRow(Result result, int rowNum) throws Exception {
+        this.resultSize += result.size();
+        return null;
+    }
+
+    public int getResultSize() {
+        return resultSize;
+    }
+}
diff --git a/commons-hbase/src/test/java/com/navercorp/pinpoint/common/hbase/bo/ColumnGetCountTest.java b/commons-hbase/src/test/java/com/navercorp/pinpoint/common/hbase/bo/ColumnGetCountTest.java
@@ -32,10 +32,7 @@ public void columnGetCountTest() {
         columnGetCount = ColumnGetCountFactory.create(Integer.MAX_VALUE);
         Assert.assertEquals(Integer.MAX_VALUE, columnGetCount.getLimit());
 
-
-        columnGetCount.setResultSize(Integer.MAX_VALUE);
-
-        Assert.assertFalse(columnGetCount.isreachedLimit());
+        Assert.assertFalse(columnGetCount.isReachedLimit(Integer.MAX_VALUE));
 
     }
 
@@ -46,10 +43,9 @@ public void columnGetCountTest2() {
         ColumnGetCount columnGetCount = ColumnGetCountFactory.create(countValue);
         Assert.assertEquals(countValue, columnGetCount.getLimit());
 
-        Assert.assertFalse(columnGetCount.isreachedLimit());
+        Assert.assertFalse(columnGetCount.isReachedLimit(0));
 
-        columnGetCount.setResultSize(++countValue);
-        Assert.assertTrue(columnGetCount.isreachedLimit());
+        Assert.assertTrue(columnGetCount.isReachedLimit(++countValue));
 
     }
 

diff --git a/web/src/main/java/com/navercorp/pinpoint/web/controller/BusinessTransactionController.java b/web/src/main/java/com/navercorp/pinpoint/web/controller/BusinessTransactionController.java
@@ -123,12 +123,8 @@ public TransactionInfoViewModel transactionInfo(@RequestParam("traceId") String
 
         RecordSet recordSet = this.transactionInfoService.createRecordSet(callTreeIterator, spanMatchFilter);
 
-        TransactionInfoViewModel result;
-        if (spanResult.getTraceState() == TraceState.State.PROGRESS && columnGetCount.isreachedLimit()) {
-            result = new TransactionInfoViewModel(transactionId, spanId, map.getNodes(), map.getLinks(), recordSet, TraceState.State.OVERFLOW, logConfiguration);
-        } else {
-            result = new TransactionInfoViewModel(transactionId, spanId, map.getNodes(), map.getLinks(), recordSet, spanResult.getTraceState(), logConfiguration);
-        }
+        TransactionInfoViewModel result = new TransactionInfoViewModel(transactionId, spanId, map.getNodes(), map.getLinks(), recordSet, spanResult.getTraceState(), logConfiguration);
+
         if (useLoadHistogramFormat) {
             result.setTimeHistogramFormat(TimeHistogramFormat.V2);
         }
@@ -180,7 +176,7 @@ public TraceViewerDataViewModel traceViewerData(@RequestParam("traceId") String
                                                     @RequestParam(value = "focusTimestamp", required = false, defaultValue = "0") long focusTimestamp,
                                                     @RequestParam(value = "agentId", required = false) String agentId,
                                                     @RequestParam(value = "spanId", required = false, defaultValue = "-1") long spanId) {
-        logger.debug("GET /traceViewerData params {traceId={}, focusTimestamp={}, agentId={}, spanId={}, v={}}", traceIdParam, focusTimestamp, agentId, spanId);
+        logger.debug("GET /traceViewerData params {traceId={}, focusTimestamp={}, agentId={}, spanId={}}", traceIdParam, focusTimestamp, agentId, spanId);
 
         final TransactionId transactionId = TransactionIdUtils.parseTransactionId(traceIdParam);
 

diff --git a/web/src/main/java/com/navercorp/pinpoint/web/dao/TraceDao.java b/web/src/main/java/com/navercorp/pinpoint/web/dao/TraceDao.java
@@ -20,6 +20,7 @@
 import com.navercorp.pinpoint.common.hbase.bo.ColumnGetCount;
 import com.navercorp.pinpoint.common.profiler.util.TransactionId;
 import com.navercorp.pinpoint.common.server.bo.SpanBo;
+import com.navercorp.pinpoint.web.service.FetchResult;
 import com.navercorp.pinpoint.web.vo.GetTraceInfo;
 
 import java.util.List;
@@ -31,7 +32,7 @@ public interface TraceDao {
 
     List<SpanBo> selectSpan(TransactionId transactionId);
 
-    List<SpanBo> selectSpan(TransactionId transactionId, ColumnGetCount columnGetCount);
+    FetchResult<List<SpanBo>> selectSpan(TransactionId transactionId, ColumnGetCount columnGetCount);
 
     List<List<SpanBo>> selectSpans(List<GetTraceInfo> getTraceInfoList);
 

diff --git a/web/src/main/java/com/navercorp/pinpoint/web/dao/hbase/HbaseTraceDaoV2.java b/web/src/main/java/com/navercorp/pinpoint/web/dao/hbase/HbaseTraceDaoV2.java
@@ -21,6 +21,7 @@
 import com.navercorp.pinpoint.common.hbase.RowMapper;
 import com.navercorp.pinpoint.common.hbase.TableNameProvider;
 import com.navercorp.pinpoint.common.hbase.bo.ColumnGetCount;
+import com.navercorp.pinpoint.common.hbase.rowmapper.ResultSizeMapper;
 import com.navercorp.pinpoint.common.hbase.rowmapper.RequestAwareDynamicRowMapper;
 import com.navercorp.pinpoint.common.hbase.rowmapper.RequestAwareRowMapper;
 import com.navercorp.pinpoint.common.hbase.rowmapper.RequestAwareRowMapperAdaptor;
@@ -36,13 +37,13 @@
 import com.navercorp.pinpoint.web.mapper.CellTraceMapper;
 import com.navercorp.pinpoint.web.mapper.SpanMapperV2;
 import com.navercorp.pinpoint.web.mapper.FilteringSpanDecoder;
+import com.navercorp.pinpoint.web.service.FetchResult;
 import com.navercorp.pinpoint.web.vo.GetTraceInfo;
 
 import org.apache.commons.collections4.CollectionUtils;
 import org.apache.commons.collections4.ListUtils;
 import org.apache.hadoop.hbase.TableName;
 import org.apache.hadoop.hbase.client.Get;
-import org.apache.hadoop.hbase.client.Result;
 import org.apache.hadoop.hbase.filter.BinaryPrefixComparator;
 import org.apache.hadoop.hbase.filter.ByteArrayComparable;
 import org.apache.hadoop.hbase.filter.ColumnCountGetFilter;
@@ -116,11 +117,11 @@ private void setup() {
 
     @Override
     public List<SpanBo> selectSpan(TransactionId transactionId) {
-        return selectSpan(transactionId, null);
+        return selectSpan(transactionId, null).getData();
     }
 
     @Override
-    public List<SpanBo> selectSpan(TransactionId transactionId, ColumnGetCount columnGetCount) {
+    public FetchResult<List<SpanBo>> selectSpan(TransactionId transactionId, ColumnGetCount columnGetCount) {
         Objects.requireNonNull(transactionId, "transactionId");
 
         byte[] transactionIdRowKey = rowKeyEncoder.encodeRowKey(transactionId);
@@ -133,17 +134,12 @@ public List<SpanBo> selectSpan(TransactionId transactionId, ColumnGetCount colum
         }
 
         TableName traceTableName = tableNameProvider.getTableName(DESCRIPTOR.getTable());
-        RowMapper<List<SpanBo>> rowMapper = new RowMapperResultAdaptor<>(spanMapperV2, new RowMapper<List<SpanBo>>() {
-            @Override
-            public List<SpanBo> mapRow(Result result, int rowNum) {
-                if (columnGetCount != null && columnGetCount != ColumnGetCount.UNLIMITED_COLUMN_GET_COUNT) {
-                    int size = result.size();
-                    columnGetCount.setResultSize(size);
-                }
-                return null;
-            }
-        });
-        return template2.get(traceTableName, get, rowMapper);
+        ResultSizeMapper<List<SpanBo>> resultSizeMapper = new ResultSizeMapper<>();
+        RowMapper<List<SpanBo>> rowMapper = new RowMapperResultAdaptor<>(spanMapperV2, resultSizeMapper);
+
+        List<SpanBo> spanBos = template2.get(traceTableName, get, rowMapper);
+
+        return new FetchResult<>(resultSizeMapper.getResultSize(), spanBos);
     }
 
     @Override

diff --git a/web/src/main/java/com/navercorp/pinpoint/web/service/FetchResult.java b/web/src/main/java/com/navercorp/pinpoint/web/service/FetchResult.java
@@ -0,0 +1,19 @@
+package com.navercorp.pinpoint.web.service;
+
+public class FetchResult<T> {
+    private final int fetchCount;
+    private final T data;
+
+    public FetchResult(int fetchCount, T data) {
+        this.fetchCount = fetchCount;
+        this.data = data;
+    }
+
+    public int getFetchCount() {
+        return fetchCount;
+    }
+
+    public T getData() {
+        return data;
+    }
+}
diff --git a/web/src/main/java/com/navercorp/pinpoint/web/service/SpanServiceImpl.java b/web/src/main/java/com/navercorp/pinpoint/web/service/SpanServiceImpl.java
@@ -117,15 +117,18 @@ public SpanResult selectSpan(TransactionId transactionId, Predicate<SpanBo> filt
         Objects.requireNonNull(transactionId, "transactionId");
         Objects.requireNonNull(filter, "filter");
 
-        final List<SpanBo> spans = traceDao.selectSpan(transactionId, columnGetCount);
+        final FetchResult<List<SpanBo>> fetchResult = traceDao.selectSpan(transactionId, columnGetCount);
+        final List<SpanBo> spans = fetchResult.getData();
         logger.debug("selectSpan spans:{}", spans.size());
 
         populateAgentName(spans);
         if (CollectionUtils.isEmpty(spans)) {
             return new SpanResult(TraceState.State.ERROR, new CallTreeIterator(null));
         }
 
-        final SpanResult result = order(spans, filter);
+        final boolean isReachedLimit = columnGetCount.isReachedLimit(fetchResult.getFetchCount());
+
+        final SpanResult result = order(spans, filter, isReachedLimit);
         final CallTreeIterator callTreeIterator = result.getCallTree();
         final List<Align> values = callTreeIterator.values();
 
@@ -488,11 +491,16 @@ public interface AnnotationReplacementCallback {
         void replacement(Align align, List<AnnotationBo> annotationBoList);
     }
 
-    private SpanResult order(List<SpanBo> spans, Predicate<SpanBo> filter) {
+    private SpanResult order(List<SpanBo> spans, Predicate<SpanBo> filter, boolean isReachedLimit) {
         SpanAligner spanAligner = new SpanAligner(spans, filter, serviceTypeRegistryService);
         final CallTree callTree = spanAligner.align();
 
-        return new SpanResult(spanAligner.getMatchType(), callTree.iterator());
+        TraceState.State matchType = spanAligner.getMatchType();
+        if (matchType == TraceState.State.PROGRESS && isReachedLimit) {
+            matchType = TraceState.State.OVERFLOW;
+        }
+
+        return new SpanResult(matchType, callTree.iterator());
     }
 
     private Optional<String> getAgentName(String agentId, long agentStartTime) {