gortiz commented on code in PR #14574: URL: https://github.com/apache/pinot/pull/14574#discussion_r1888366484
########## pinot-broker/src/main/java/org/apache/pinot/broker/requesthandler/MultiStageQueryThrottler.java: ########## @@ -0,0 +1,168 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.pinot.broker.requesthandler; + +import com.google.common.annotations.VisibleForTesting; +import com.google.common.base.Preconditions; +import java.util.Collections; +import java.util.List; +import java.util.concurrent.TimeUnit; +import org.apache.helix.HelixAdmin; +import org.apache.helix.HelixConstants; +import org.apache.helix.HelixManager; +import org.apache.helix.model.HelixConfigScope; +import org.apache.helix.model.builder.HelixConfigScopeBuilder; +import org.apache.pinot.broker.broker.helix.ClusterChangeHandler; +import org.apache.pinot.common.concurrency.AdjustableSemaphore; +import org.apache.pinot.spi.utils.CommonConstants; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + + +/** + * This class helps limit the number of multi-stage queries being executed concurrently. Currently, this limit is + * applied at the broker level, but could be moved to the server level in the future. Note that the cluster + * configuration is a "per server" value and the broker currently simply assumes that a query will be across all + * servers. Another assumption here is that queries are evenly distributed across brokers. Ideally, we want to move to a + * model where the broker asks each server whether it can execute a query stage before dispatching the query stage to + * the server. This would allow for more fine-grained control over the number of queries being executed concurrently + * (but there are some limitations around ordering and blocking that need to be solved first). Review Comment: nit: I think we are merging two different things here. In my mind there are 3 options: 1. Broader broker check, the one we are implementing, which simplifies the check by assuming evenly distributed queries across brokers and servers. 2. Ask servers if they can execute queries, where the broker ask the servers whether they can execute more queries or not (probably this should be done while workers/plan fragments are assigned). A server needs to be able to execute all the associated workers of the query atomically. This means that a query that spawns a lot of workers on a single server may consume a lot of threads in the server. 3. Ask servers if they can execute workers. This is what implies there could be blocks if we don't care about ordering. The difference between 2 and 3 is that queries that require tons of workers may end up consuming a lot of threads in 2, while in 3 we would execute only the some workers and once they finish the other will start to run. In order to do so we need to execute the dag in a consistent manner (and probably buffer some data). -- 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]
