[GitHub] [druid] suneet-s commented on a change in pull request #11077: Add retry around query loop in ITWikipediaQueryTest.testQueryLaningLaneIsLimited

[GitBox <gi...@apache.org>]

suneet-s commented on a change in pull request #11077:
URL: https://github.com/apache/druid/pull/11077#discussion_r609142406



##########
File path: integration-tests/src/test/java/org/apache/druid/tests/query/ITWikipediaQueryTest.java
##########
@@ -85,37 +88,54 @@ public void testWikipediaQueriesFromFile() throws Exception
   @Test
   public void testQueryLaningLaneIsLimited() throws Exception
   {
-    // the broker is configured with a manually defined query lane, 'one' with limit 1
-    //  -Ddruid.query.scheduler.laning.type=manual
-    //  -Ddruid.query.scheduler.laning.lanes.one=1
-    // by issuing 50 queries, at least 1 of them will succeed on 'one', and at least 1 of them will overlap enough to
-    // get limited
-    final int numQueries = 50;
-    List<Future<StatusResponseHolder>> futures = new ArrayList<>(numQueries);
-    for (int i = 0; i < numQueries; i++) {
-      futures.add(
-          queryClient.queryAsync(
-              queryHelper.getQueryURL(config.getBrokerUrl()),
-              getQueryBuilder().build()
-          )
-      );
-    }
-
-    int success = 0;
-    int limited = 0;
-
-    for (Future<StatusResponseHolder> future : futures) {
-      StatusResponseHolder status = future.get();
-      if (status.getStatus().getCode() == QueryCapacityExceededException.STATUS_CODE) {
-        limited++;
-        Assert.assertTrue(status.getContent().contains(QueryCapacityExceededException.makeLaneErrorMessage("one", 1)));
-      } else if (status.getStatus().getCode() == HttpResponseStatus.OK.getCode()) {
-        success++;
-      }
-    }
-
-    Assert.assertTrue(success > 0);
-    Assert.assertTrue(limited > 0);
+    ITRetryUtil.retryUntil(
+        () -> {
+          // the broker is configured with a manually defined query lane, 'one' with limit 1
+          //  -Ddruid.query.scheduler.laning.type=manual
+          //  -Ddruid.query.scheduler.laning.lanes.one=1
+          // by issuing 50 queries, at least 1 of them will succeed on 'one', and at least 1 of them will overlap enough to
+          // get limited.

Review comment:
       Have you considered introducing an artificial sleep on the query execution path for queries issued by this test? 
   
   I think we can accomplish this with what we have today by using one of the javascript features (aggregators, filters, etc.) available to us.
   
   This way we can guarantee overlaps, instead of having to rely on a re-try mechanism? 




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



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