bziobrowski commented on code in PR #14226:
URL: https://github.com/apache/pinot/pull/14226#discussion_r1799591241
##########
pinot-broker/src/test/java/org/apache/pinot/broker/queryquota/HelixExternalViewBasedQueryQuotaManagerTest.java:
##########
@@ -531,25 +674,41 @@ private static ExternalView generateBrokerResource(String
tableName) {
private void runQueries()
throws InterruptedException {
runQueries(TABLE_MAX_QPS, false);
- //increase the qps and some of the queries should be throttled.
- runQueries(TABLE_MAX_QPS * 2, true);
+ // increase the qps and some of the queries should be throttled.
+ // keep in mind that permits are 'regenerated' on every call based on how
much time elapsed since last one
+ // that means for 25 QPS we get new permit every 40 ms or 0.5 every 20 ms
+ // if we start with 25 permits at time t1 then if we want to exceed the
qps in the next second we've to do more
+ // double requests, because 25 will regenerate
+ runQueries(TABLE_MAX_QPS * 2 + 1, true);
+ }
+
+ private void runQueries(double qps, boolean shouldFail)
+ throws InterruptedException {
+ runQueries(qps, shouldFail, APP_NAME);
}
// try to keep the qps below 50 to ensure that the time lost between 2 query
runs on top of the sleepMillis
// is not comparable to sleepMillis, else the actual qps would end being lot
lower than required qps
- private void runQueries(double qps, boolean shouldFail)
+ private void runQueries(double qps, boolean shouldFail, String appName)
throws InterruptedException {
int failCount = 0;
long sleepMillis = (long) (1000 / qps);
for (int i = 0; i < qps; i++) {
+ if (!_queryQuotaManager.acquireApplication(appName)) {
+ failCount++;
+ }
if
(!_queryQuotaManager.acquireDatabase(CommonConstants.DEFAULT_DATABASE)) {
failCount++;
}
if (!_queryQuotaManager.acquire(RAW_TABLE_NAME)) {
failCount++;
}
- Thread.sleep(sleepMillis);
Review Comment:
I replaced sleeps with timesource.advanceByMillis(x) to make test more
stable (because sleeping could be affected by GC or random interrupts) and much
faster. I'd be cool if similar approach could be used in integration test.
##########
pinot-broker/src/test/java/org/apache/pinot/broker/queryquota/HelixExternalViewBasedQueryQuotaManagerTest.java:
##########
@@ -531,25 +674,41 @@ private static ExternalView generateBrokerResource(String
tableName) {
private void runQueries()
throws InterruptedException {
runQueries(TABLE_MAX_QPS, false);
- //increase the qps and some of the queries should be throttled.
- runQueries(TABLE_MAX_QPS * 2, true);
+ // increase the qps and some of the queries should be throttled.
+ // keep in mind that permits are 'regenerated' on every call based on how
much time elapsed since last one
+ // that means for 25 QPS we get new permit every 40 ms or 0.5 every 20 ms
+ // if we start with 25 permits at time t1 then if we want to exceed the
qps in the next second we've to do more
+ // double requests, because 25 will regenerate
+ runQueries(TABLE_MAX_QPS * 2 + 1, true);
+ }
+
+ private void runQueries(double qps, boolean shouldFail)
+ throws InterruptedException {
+ runQueries(qps, shouldFail, APP_NAME);
}
// try to keep the qps below 50 to ensure that the time lost between 2 query
runs on top of the sleepMillis
// is not comparable to sleepMillis, else the actual qps would end being lot
lower than required qps
- private void runQueries(double qps, boolean shouldFail)
+ private void runQueries(double qps, boolean shouldFail, String appName)
throws InterruptedException {
int failCount = 0;
long sleepMillis = (long) (1000 / qps);
for (int i = 0; i < qps; i++) {
+ if (!_queryQuotaManager.acquireApplication(appName)) {
+ failCount++;
+ }
if
(!_queryQuotaManager.acquireDatabase(CommonConstants.DEFAULT_DATABASE)) {
failCount++;
}
if (!_queryQuotaManager.acquire(RAW_TABLE_NAME)) {
failCount++;
}
- Thread.sleep(sleepMillis);
Review Comment:
I replaced sleeps with timesource.advanceByMillis(x) to make test more
stable (because sleeping could be affected by GC delays or random interrupts)
and much faster. I'd be cool if similar approach could be used in integration
test.
--
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-unsubscr...@pinot.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
