You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by GitBox <gi...@apache.org> on 2022/04/14 10:31:04 UTC

[GitHub] [pinot] richardstartin commented on a diff in pull request #8539: Add brokerURL and better handle TimeoutException in QueryRunner

richardstartin commented on code in PR #8539:
URL: https://github.com/apache/pinot/pull/8539#discussion_r850312126


##########
pinot-tools/src/main/java/org/apache/pinot/tools/perf/QueryRunner.java:
##########
@@ -416,42 +422,49 @@ public static QuerySummary multiThreadedQueryRunner(PerfBenchmarkDriverConf conf
     long reportStartTime = startTime;
     int numReportIntervals = 0;
     int numTimesExecuted = 0;
-    while (numTimesToRunQueries == 0 || numTimesExecuted < numTimesToRunQueries) {
-      if (executorService.isTerminated()) {
-        throw new IllegalThreadStateException("All threads got exception and already dead.");
-      }
-
-      if (timeout > 0 && System.currentTimeMillis() - startTimeAbsolute > timeout) {
-        throw new TimeoutException("Timeout of " + timeout + " sec reached. Aborting");
-      }
+    try {
+      while (numTimesToRunQueries == 0 || numTimesExecuted < numTimesToRunQueries) {
+        if (executorService.isTerminated()) {
+          throw new IllegalThreadStateException("All threads got exception and already dead.");
+        }
 
-      for (String query : queries) {
-        while (!queryQueue.offer(query)) {
-          Thread.sleep(1);
+        if (timeout > 0 && System.currentTimeMillis() - startTimeAbsolute > timeout) {
+          throw new TimeoutException("Timeout of " + timeout + " sec reached. Aborting");
         }
 
-        long currentTime = System.currentTimeMillis();
-        if (currentTime - reportStartTime >= reportIntervalMs) {
-          long timePassed = currentTime - startTime;
-          int numQueriesExecutedInt = numQueriesExecuted.get();
-          LOGGER.info("Time Passed: {}ms, Queries Executed: {}, Exceptions: {}, Average QPS: {}, "
-                  + "Average Broker Time: {}ms, Average Client Time: {}ms.", timePassed, numQueriesExecutedInt,
-              numExceptions.get(), numQueriesExecutedInt / ((double) timePassed / MILLIS_PER_SECOND),
-              totalBrokerTime.get() / (double) numQueriesExecutedInt,
-              totalClientTime.get() / (double) numQueriesExecutedInt);
-          reportStartTime = currentTime;
-          numReportIntervals++;
-
-          if ((numIntervalsToReportAndClearStatistics != 0) && (numReportIntervals
-              == numIntervalsToReportAndClearStatistics)) {
-            numReportIntervals = 0;
-            startTime = currentTime;
-            reportAndClearStatistics(numQueriesExecuted, numExceptions, totalBrokerTime, totalClientTime,
-                statisticsList);
+        for (String query : queries) {
+          if (timeout > 0 && System.currentTimeMillis() - startTimeAbsolute > timeout) {
+            throw new TimeoutException("Timeout of " + timeout + " sec reached. Aborting");
+          }
+          while (!queryQueue.offer(query)) {
+            Thread.sleep(1);
+          }
+
+          long currentTime = System.currentTimeMillis();
+          if (currentTime - reportStartTime >= reportIntervalMs) {
+            long timePassed = currentTime - startTime;
+            int numQueriesExecutedInt = numQueriesExecuted.get();
+            LOGGER.info("Time Passed: {}ms, Queries Executed: {}, Exceptions: {}, Average QPS: {}, " + "Average "
+                    + "Broker Time: {}ms, Average Client Time: {}ms.", timePassed, numQueriesExecutedInt,
+                numExceptions.get(), numQueriesExecutedInt / ((double) timePassed / MILLIS_PER_SECOND),
+                totalBrokerTime.get() / (double) numQueriesExecutedInt,
+                totalClientTime.get() / (double) numQueriesExecutedInt);
+            reportStartTime = currentTime;
+            numReportIntervals++;
+
+            if ((numIntervalsToReportAndClearStatistics != 0) && (numReportIntervals
+                == numIntervalsToReportAndClearStatistics)) {
+              numReportIntervals = 0;
+              startTime = currentTime;
+              reportAndClearStatistics(numQueriesExecuted, numExceptions, totalBrokerTime, totalClientTime,
+                  statisticsList);
+            }
           }
         }
+        numTimesExecuted++;
       }
-      numTimesExecuted++;
+    } catch (TimeoutException e) {
+      LOGGER.info("Time out reached. {}", e.getMessage());

Review Comment:
   timeouts should not be omitted from the reported statistics
   



-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org