snleee commented on a change in pull request #6898:
URL: https://github.com/apache/incubator-pinot/pull/6898#discussion_r630550564
##########
File path:
pinot-broker/src/main/java/org/apache/pinot/broker/queryquota/HelixExternalViewBasedQueryQuotaManager.java
##########
@@ -194,15 +195,30 @@ private void createRateLimiter(String tableNameWithType,
ExternalView brokerReso
// Get stat from property store
String tableConfigPath = constructTableConfigPath(tableNameWithType);
Stat stat = _propertyStore.getStat(tableConfigPath,
AccessOption.PERSISTENT);
-
double perBrokerRate = overallRate / onlineCount;
- QueryQuotaEntity queryQuotaEntity =
- new QueryQuotaEntity(RateLimiter.create(perBrokerRate), new
HitCounter(ONE_SECOND_TIME_RANGE_IN_SECOND),
- new MaxHitRateTracker(ONE_MINUTE_TIME_RANGE_IN_SECOND),
onlineCount, overallRate, stat.getVersion());
- _rateLimiterMap.put(tableNameWithType, queryQuotaEntity);
- LOGGER.info(
- "Rate limiter for table: {} has been initialized. Overall rate: {}.
Per-broker rate: {}. Number of online broker instances: {}. Table config stat
version: {}",
- tableNameWithType, overallRate, perBrokerRate, onlineCount,
stat.getVersion());
+
+ QueryQuotaEntity queryQuotaEntity = _rateLimiterMap.get(tableNameWithType);
+ if (queryQuotaEntity == null) {
+ queryQuotaEntity =
+ new QueryQuotaEntity(RateLimiter.create(perBrokerRate), new
HitCounter(ONE_SECOND_TIME_RANGE_IN_SECOND),
+ new MaxHitRateTracker(ONE_MINUTE_TIME_RANGE_IN_SECOND),
onlineCount, overallRate, stat.getVersion());
+ _rateLimiterMap.put(tableNameWithType, queryQuotaEntity);
+ LOGGER.info(
+ "Rate limiter for table: {} has been initialized. Overall rate: {}.
Per-broker rate: {}. Number of online broker instances: {}. Table config stat
version: {}",
+ tableNameWithType, overallRate, perBrokerRate, onlineCount,
stat.getVersion());
+ } else {
+ queryQuotaEntity.getRateLimiter().setRate(perBrokerRate);
+ queryQuotaEntity.setNumOnlineBrokers(onlineCount);
+ queryQuotaEntity.setOverallRate(overallRate);
+ queryQuotaEntity.setTableConfigStatVersion(stat.getVersion());
+ LOGGER.info(
+ "Rate limiter for table: {} has been updated. Overall rate: {}.
Per-broker rate: {}. Number of online broker instances: {}. Table config stat
version: {}",
+ tableNameWithType, overallRate, perBrokerRate, onlineCount,
stat.getVersion());
+ }
+
+ final QueryQuotaEntity finalQueryQuotaEntity = queryQuotaEntity;
+ _brokerMetrics.addCallbackTableGaugeIfNeeded(tableNameWithType,
BrokerGauge.MAX_BURST_QPS,
Review comment:
Why do we use `addCallbackTableGaugeIfNeeded` instead of
`setValueOfTableGauge`? Can you provide me some context here?
##########
File path:
pinot-broker/src/test/java/org/apache/pinot/broker/queryquota/HelixExternalViewBasedQueryQuotaManagerTest.java
##########
@@ -149,7 +149,7 @@ public void testOfflineTableNotnullQuota()
ZKMetadataProvider
.setOfflineTableConfig(_testPropertyStore, OFFLINE_TABLE_NAME,
TableConfigUtils.toZNRecord(tableConfig));
setQps(tableConfig);
- _queryQuotaManager.initTableQueryQuota(tableConfig, brokerResource);
+ _queryQuotaManager.initOrUpdateTableQueryQuota(tableConfig,
brokerResource);
Review comment:
Can we add the test case that's covering the following case?
1. Create ratelimiter
2. run some acquire
3. Check brokerMetrics table gauge value
4. Change the ratelimiter config
4. run some acquire
5. Check brokerMetrics table gauge value
--
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:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
