-
Notifications
You must be signed in to change notification settings - Fork 3.8k
add manual laning strategy, integration test #9492
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
6654a72
b187125
700e6ab
13491bb
a05c9fa
20c10cf
0bf4232
ee435ad
cdb2d12
d976269
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -19,16 +19,31 @@ | |
|
|
||
| package org.apache.druid.tests.query; | ||
|
|
||
| import com.google.common.collect.ImmutableMap; | ||
| import com.google.inject.Inject; | ||
| import org.apache.druid.java.util.common.StringUtils; | ||
| import org.apache.druid.java.util.http.client.response.StatusResponseHolder; | ||
| import org.apache.druid.query.Druids; | ||
| import org.apache.druid.query.aggregation.CountAggregatorFactory; | ||
| import org.apache.druid.server.QueryCapacityExceededException; | ||
| import org.apache.druid.testing.IntegrationTestingConfig; | ||
| import org.apache.druid.testing.clients.CoordinatorResourceTestClient; | ||
| import org.apache.druid.testing.clients.QueryResourceTestClient; | ||
| import org.apache.druid.testing.guice.DruidTestModuleFactory; | ||
| import org.apache.druid.testing.utils.ITRetryUtil; | ||
| import org.apache.druid.testing.utils.TestQueryHelper; | ||
| import org.apache.druid.tests.TestNGGroup; | ||
| import org.jboss.netty.handler.codec.http.HttpResponseStatus; | ||
| import org.testng.Assert; | ||
| import org.testng.annotations.BeforeMethod; | ||
| import org.testng.annotations.Guice; | ||
| import org.testng.annotations.Test; | ||
|
|
||
| import java.util.ArrayList; | ||
| import java.util.List; | ||
| import java.util.UUID; | ||
| import java.util.concurrent.Future; | ||
|
|
||
| @Test(groups = TestNGGroup.QUERY) | ||
| @Guice(moduleFactory = DruidTestModuleFactory.class) | ||
| public class ITWikipediaQueryTest | ||
|
|
@@ -42,6 +57,10 @@ public class ITWikipediaQueryTest | |
| private CoordinatorResourceTestClient coordinatorClient; | ||
| @Inject | ||
| private TestQueryHelper queryHelper; | ||
| @Inject | ||
| private QueryResourceTestClient queryClient; | ||
| @Inject | ||
| private IntegrationTestingConfig config; | ||
|
|
||
| @BeforeMethod | ||
| public void before() throws Exception | ||
|
|
@@ -51,15 +70,112 @@ public void before() throws Exception | |
| ITRetryUtil.retryUntilTrue( | ||
| () -> coordinatorClient.areSegmentsLoaded(WIKIPEDIA_DATA_SOURCE), "wikipedia segment load" | ||
| ); | ||
| coordinatorClient.initializeLookups(WIKIPEDIA_LOOKUP_RESOURCE); | ||
| ITRetryUtil.retryUntilTrue( | ||
| () -> coordinatorClient.areLookupsLoaded(WIKI_LOOKUP), "wikipedia lookup load" | ||
| ); | ||
| if (!coordinatorClient.areLookupsLoaded(WIKI_LOOKUP)) { | ||
| coordinatorClient.initializeLookups(WIKIPEDIA_LOOKUP_RESOURCE); | ||
| ITRetryUtil.retryUntilTrue( | ||
| () -> coordinatorClient.areLookupsLoaded(WIKI_LOOKUP), "wikipedia lookup load" | ||
| ); | ||
| } | ||
| } | ||
|
|
||
| @Test | ||
| public void testWikipediaQueriesFromFile() throws Exception | ||
| { | ||
| queryHelper.testQueriesFromFile(WIKIPEDIA_QUERIES_RESOURCE, 2); | ||
| } | ||
|
|
||
| @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(StringUtils.format(QueryCapacityExceededException.ERROR_MESSAGE_TEMPLATE, "one"))); | ||
| } else if (status.getStatus().getCode() == HttpResponseStatus.OK.getCode()) { | ||
| success++; | ||
| } | ||
| } | ||
|
|
||
| Assert.assertTrue(success > 0); | ||
| Assert.assertTrue(limited > 0); | ||
|
|
||
| // test another to make sure we can still issue one query at a time | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This can be separate into another @test method. Will make understanding each test case easier.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hmm, this is testing a sequence of events though, so it belongs in the same test
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I was thinking the top part is test where we exceed and expect rejection. The bottom part is test where we are always under the limit and never exceed hence expect all to be successful.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. added a separate test which runs a bunch of queries the same manner that do not belong to a lane to make sure they are not limited |
||
| StatusResponseHolder followUp = queryClient.queryAsync( | ||
| queryHelper.getQueryURL(config.getBrokerUrl()), | ||
| getQueryBuilder().build() | ||
| ).get(); | ||
|
|
||
| Assert.assertEquals(HttpResponseStatus.OK.getCode(), followUp.getStatus().getCode()); | ||
|
|
||
| StatusResponseHolder andAnother = queryClient.queryAsync( | ||
| queryHelper.getQueryURL(config.getBrokerUrl()), | ||
| getQueryBuilder().build() | ||
| ).get(); | ||
|
|
||
| Assert.assertEquals(HttpResponseStatus.OK.getCode(), andAnother.getStatus().getCode()); | ||
| } | ||
|
|
||
| @Test | ||
| public void testQueryLaningWithNoLane() 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 | ||
| // these queries will not belong to the lane so none of them should be 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().context(ImmutableMap.of("queryId", UUID.randomUUID().toString())).build() | ||
| ) | ||
| ); | ||
| } | ||
|
|
||
| int success = 0; | ||
| int limited = 0; | ||
|
|
||
| for (Future<StatusResponseHolder> future : futures) { | ||
| StatusResponseHolder status = future.get(); | ||
| if (status.getStatus().getCode() == QueryCapacityExceededException.STATUS_CODE) { | ||
| limited++; | ||
| } else if (status.getStatus().getCode() == HttpResponseStatus.OK.getCode()) { | ||
| success++; | ||
| } | ||
| } | ||
|
|
||
| Assert.assertTrue(success > 0); | ||
| Assert.assertEquals(limited, 0); | ||
|
|
||
| } | ||
|
|
||
| private Druids.TimeseriesQueryBuilder getQueryBuilder() | ||
| { | ||
| return Druids.newTimeseriesQueryBuilder() | ||
| .dataSource("wikipedia_editstream") | ||
| .aggregators(new CountAggregatorFactory("chocula")) | ||
| .intervals("2013-01-01T00:00:00.000/2013-01-08T00:00:00.000") | ||
| .context(ImmutableMap.of("lane", "one", "queryId", UUID.randomUUID().toString())); | ||
| } | ||
| } | ||
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we also have a test for when the Manual Lane From Context That Arent In Map is given? For example, if lane in query context is "some-unknown-lane"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess they will always be rejected since we give NO_CAPACITY right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
^ update typo above. Meant to say "also" have a test for case when the Manual Lane From Context That Arent In Map is given