ankitsultana commented on code in PR #15203: URL: https://github.com/apache/pinot/pull/15203#discussion_r2082785248
########## pinot-broker/src/main/java/org/apache/pinot/broker/routing/adaptiveserverselector/PriorityGroupInstanceSelector.java: ########## @@ -0,0 +1,229 @@ +/** + * 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.routing.adaptiveserverselector; + +import java.util.ArrayList; +import java.util.Collections; +import java.util.HashMap; +import java.util.HashSet; +import java.util.List; +import java.util.Map; +import java.util.Set; +import java.util.stream.Collectors; +import javax.annotation.Nullable; +import org.apache.commons.lang3.tuple.ImmutablePair; +import org.apache.commons.lang3.tuple.Pair; +import org.apache.pinot.broker.routing.instanceselector.SegmentInstanceCandidate; + + +/** + * A server selector that implements priority-based server selection based on replica groups. + * This selector works in conjunction with an {@link AdaptiveServerSelector} to provide + * a two-level selection strategy: + * <ol> + * <li>First level: Select servers based on replica group priorities</li> + * <li>Second level: Use adaptive selection within the chosen replica group</li> + * </ol> + * + * <p>The selector maintains the following invariants:</p> + * <ul> + * <li>Servers from preferred replica groups are always selected before non-preferred groups</li> + * <li>Within each replica group, servers are selected using adaptive selection criteria</li> + * <li>When no preferred groups are available, falls back to non-preferred groups</li> + * </ul> + */ +public class PriorityGroupInstanceSelector { + + /** The underlying adaptive server selector used for selection within replica groups */ + private final AdaptiveServerSelector _adaptiveServerSelector; Review Comment: We are tying this feature to adaptive server selector. But for certain use-cases we may have to disable it (e.g. for colocated joins). ########## pinot-broker/src/main/java/org/apache/pinot/broker/routing/instanceselector/MultiStageReplicaGroupSelector.java: ########## @@ -65,9 +66,10 @@ public MultiStageReplicaGroupSelector(String tableNameWithType, ZkHelixPropertyS } @Override - public void init(Set<String> enabledInstances, IdealState idealState, ExternalView externalView, - Set<String> onlineSegments) { - super.init(enabledInstances, idealState, externalView, onlineSegments); + public void init(Set<String> enabledInstances, Map<String, ServerInstance> enabledServerMap, + IdealState idealState, ExternalView externalView, Review Comment: nit: format ########## pinot-common/src/main/java/org/apache/pinot/common/metrics/BrokerMeter.java: ########## @@ -50,6 +50,18 @@ public class BrokerMeter implements AbstractMetrics.Meter { * That means it can be used to know how many single-stage queries have been started in total. */ public static final BrokerMeter QUERIES_GLOBAL = create("QUERIES_GLOBAL", "queries", true); + /** + * Number of queries executed, this metric is not global and is attached to a particular replica group. + * <p> + * At this moment, this counter include single stage queries only. + */ + public static final BrokerMeter REPLICA_QUERIES = create("REPLICA_QUERIES", "routing", false); + /** + * Number of segment queries executed, this metric is not global and is attached to a particular replica group. + * <p> + * At this moment, this counter include single stage queries only. + */ + public static final BrokerMeter REPLICA_SEG_QUERIES = create("REPLICA_SEG_QUERIES", "routing", false); Review Comment: What are segment queries? If this is not that important we can skip it to be minimal. I suppose the other metric should suffice ########## pinot-broker/src/main/java/org/apache/pinot/broker/routing/adaptiveserverselector/ServerSelectionContext.java: ########## @@ -0,0 +1,56 @@ +/** + * 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.routing.adaptiveserverselector; + +import java.util.Collections; +import java.util.List; +import java.util.Map; +import org.apache.pinot.common.utils.config.QueryOptionsUtils; + + +/** + * This class encapsulates query options and ordered preferred replica groups that influence how + * servers are selected for query execution. + */ +public class ServerSelectionContext { + // Multiple query options can be used to influence server selection. Passed into the context class to avoid endless constructor arguments changes. + private final Map<String, String> _queryOptions; + // If some query options need further processing, store the parsing result here to avoid duplicate parsing. + private final List<Integer> _orderedPreferredGroups; + + /** + * Creates a new server selection context with the given query options. + * The ordered preferred groups are extracted from the query options using + * {@link QueryOptionsUtils#getOrderedPreferredReplicas(Map)}. + * + * @param queryOptions map of query options that may contain server selection preferences + */ + public ServerSelectionContext(Map<String, String> queryOptions) { + _queryOptions = queryOptions == null ? Collections.emptyMap() : queryOptions; + _orderedPreferredGroups = QueryOptionsUtils.getOrderedPreferredReplicas(_queryOptions); + } + + public Map<String, String> getQueryOptions() { Review Comment: I see this is unused. I think we can avoid this class and directly pass in the preferredGroups around. The name of this class kinda suggests that this context is available throughout server selection, but that's not really true and it's usefulness is very limited right now. -- 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
