yashmayya opened a new pull request, #20099: URL: https://github.com/apache/pinot/pull/20099
## Problem `MultiStageQueryThrottler` gives each broker a pool of `maxServerQueryThreads * numServers / numBrokers` permits. It has two gaps: 1. It counts every `Broker_` instance config. A broker that serves no table gets no queries, but it still makes every other broker's pool smaller. 2. A query estimated above the pool fails at once. The pool depends on the broker count, so adding brokers can make a query fail that passed before. ## Change - Count brokers from the `brokerResource` IdealState. A broker that serves no table is not counted. Restarts do not change this count, so the pool does not change during a rolling restart. - Cap each query's estimate at the pool size. A query above the pool now waits until it can take the whole pool, then runs alone. It no longer fails. - The `LOG` strategy now compares against the pool. Before, it compared against the per-server limit. - Skip the external view work when throttling is off. There are no new configs. ## Trade-offs - A capped query waits for the running MSE queries on its broker to finish. The semaphore is fair, so later queries wait behind it. The query timeout limits the wait. Master already does this for queries near the pool size. - If the pool gets smaller while a capped query waits (a broker added, a server removed), that query can wait until it times out. - While throttling is on, each external view callback reads the `brokerResource` IdealState once. - A query with many partitions can need more threads than the pool. A `partition_size` hint that divides the partition count reduces the thread count. For example, 64 partitions on 8 servers need 384 threads without the hint and 72 threads with `partition_size=8`. ## Tests - `MultiStageQueryThrottlerTest`: the broker count comes from the IdealState, a broker that serves no table is not counted, an oversize query is capped instead of rejected, and the `LOG` test now calls `init`. - `MultiStageBrokerRequestHandlerTest`: the handler acquires and releases the capped estimate. -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
