shounakmk219 commented on code in PR #13544: URL: https://github.com/apache/pinot/pull/13544#discussion_r1677931484
########## pinot-broker/src/main/java/org/apache/pinot/broker/queryquota/HelixExternalViewBasedQueryQuotaManager.java: ########## @@ -230,6 +248,72 @@ private void createOrUpdateRateLimiter(String tableNameWithType, ExternalView br } } + /** + * Updates the database rate limiter if it already exists. Will not create a new database rate limiter. + * @param databaseName database name for which rate limiter needs to be updated + */ + public void updateDatabaseRateLimiter(String databaseName) { + if (!_databaseRateLimiterMap.containsKey(databaseName)) { + return; + } + createOrUpdateDatabaseRateLimiter(Collections.singletonList(databaseName)); + } + + private void createOrUpdateDatabaseRateLimiter(List<String> databaseNames) { + double databaseQpsQuota = _defaultQpsQuotaForDatabase; + ExternalView brokerResource = HelixHelper + .getExternalViewForResource(_helixManager.getClusterManagmentTool(), _helixManager.getClusterName(), + CommonConstants.Helix.BROKER_RESOURCE_INSTANCE); + // Tables in database can span across broker tags as we don't maintain a broker tag to database mapping as of now. + // Hence, we consider all online brokers for the rate distribution. + int onlineBrokers = HelixHelper.getOnlineInstanceFromExternalView(brokerResource).size(); + for (String databaseName : databaseNames) { + DatabaseConfig databaseConfig = + ZKMetadataProvider.getDatabaseConfig(_helixManager.getHelixPropertyStore(), databaseName); + if (databaseConfig != null && databaseConfig.getQuotaConfig() != null + && databaseConfig.getQuotaConfig().getMaxQPS() != -1) { + databaseQpsQuota = databaseConfig.getQuotaConfig().getMaxQPS(); + } + if (databaseQpsQuota < 0) { + buildEmptyOrResetRateLimiterInDatabaseQueryQuotaEntity(databaseName); + continue; + } + double perBrokerQpsQuota = databaseQpsQuota / onlineBrokers; Review Comment: You are absolutely right. Right now there is no defined relation between database and broker tenant which complicates the per broker rate calculation. We can apply different strategies here like get all the broker tenants under a database and only distribute quota among those brokers. But again what if tenant A get 90% of queries and tenant B tables get 10% of queries but tables tagged to tenant B are part of a bigger tenant so have more number of brokers tagged to them leading to less effective quota for tenant A brokers, we still end up giving less quota to the brokers with majority query load. Point being there is no right solution here. Based on how the user is leveraging the database concept with tenants, different strategies makes sense. The current strategy which equally distributes the quota equally across all brokers works well for a setup which has just 1 tenant (this would be the case most of the time). Starting with this for now and we can have a follow-up item to accommodate different quota distribution strategies as the database feature gets more used and new requirements come up. -- 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