You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@storm.apache.org by GitBox <gi...@apache.org> on 2020/06/02 18:36:37 UTC

[GitHub] [storm] Ethanlm commented on a change in pull request #3277: STORM-3641 upgrade metrics API for JCQueue

Ethanlm commented on a change in pull request #3277:
URL: https://github.com/apache/storm/pull/3277#discussion_r434013384



##########
File path: storm-client/src/jvm/org/apache/storm/daemon/worker/WorkerTransfer.java
##########
@@ -65,8 +67,10 @@
                                                + Config.TOPOLOGY_TRANSFER_BUFFER_SIZE + ":" + xferQueueSz);
         }
 
+        List<Integer> taskIds = new ArrayList<>();
+        taskIds.add(-1);
         this.transferQueue = new JCQueue("worker-transfer-queue", xferQueueSz, 0, xferBatchSz, backPressureWaitStrategy,
-            workerState.getTopologyId(), Constants.SYSTEM_COMPONENT_ID, -1, workerState.getPort(),
+            workerState.getTopologyId(), Constants.SYSTEM_COMPONENT_ID, taskIds, workerState.getPort(),

Review comment:
       can be simplified with `Collections.singletonList(-1)`

##########
File path: examples/storm-perf/src/main/java/org/apache/storm/perf/queuetest/JCQueuePerfTest.java
##########
@@ -45,11 +47,17 @@ public static void main(String[] args) throws Exception {
 
     }
 
+    private static List<Integer> getTaskIds() {
+        List<Integer> taskIds = new ArrayList<>();
+        taskIds.add(1000);
+        return taskIds;
+    }
+
     private static void ackingProducerSimulation() {
         WaitStrategyPark ws = new WaitStrategyPark(100);
         StormMetricRegistry registry = new StormMetricRegistry();
-        JCQueue spoutQ = new JCQueue("spoutQ", 1024, 0, 100, ws, "test", "test", 1000, 1000, registry);
-        JCQueue ackQ = new JCQueue("ackQ", 1024, 0, 100, ws, "test", "test", 1000, 1000, registry);
+        JCQueue spoutQ = new JCQueue("spoutQ", 1024, 0, 100, ws, "test", "test", getTaskIds(), 1000, registry);

Review comment:
       Can be simplified with `Collections.singletonList(100)`

##########
File path: storm-client/test/jvm/org/apache/storm/utils/JCQueueBackpressureTest.java
##########
@@ -18,11 +18,16 @@
 import org.junit.Assert;
 import org.junit.Test;
 
+import java.util.ArrayList;
+import java.util.List;
+
 
 public class JCQueueBackpressureTest {
     
     private static JCQueue createQueue(String name, int queueSize) {
-        return new JCQueue(name, queueSize, 0, 1, new WaitStrategyPark(0), "test", "test", 1000, 1000, new StormMetricRegistry());
+        List<Integer> taskIds = new ArrayList<>();
+        taskIds.add(1000);
+        return new JCQueue(name, queueSize, 0, 1, new WaitStrategyPark(0), "test", "test", taskIds, 1000, new StormMetricRegistry());

Review comment:
       can be simplified with `Collections.singletonList(1000)`

##########
File path: storm-client/test/jvm/org/apache/storm/utils/JCQueueTest.java
##########
@@ -157,7 +159,9 @@ private JCQueue createQueue(String name, int queueSize) {
     }
 
     private JCQueue createQueue(String name, int batchSize, int queueSize) {
-        return new JCQueue(name, queueSize, 0, batchSize, waitStrategy, "test", "test", 1000, 1000, new StormMetricRegistry());
+        List<Integer> taskIds = new ArrayList<>();
+        taskIds.add(1000);

Review comment:
       here too




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org