yashmayya commented on code in PR #16083:
URL: https://github.com/apache/pinot/pull/16083#discussion_r2141505775
##########
pinot-query-planner/src/main/java/org/apache/pinot/query/routing/WorkerManager.java:
##########
@@ -319,48 +340,44 @@ private boolean
isPrePartitionAssignment(List<PlanFragment> children,
/**
* Returns the servers serving any segment of the tables in the query.
*/
- private List<ServerInstance> assignServerInstances(DispatchablePlanContext
context) {
+ private List<ServerInstance> getCandidateServers(DispatchablePlanContext
context) {
List<ServerInstance> serverInstances;
Set<String> tableNames = context.getTableNames();
+ assert tableNames != null;
Map<String, ServerInstance> enabledServerInstanceMap =
_routingManager.getEnabledServerInstanceMap();
- if (tableNames.isEmpty()) {
- // TODO: Short circuit it when no table needs to be scanned
- // This could be the case from queries that don't actually fetch values
from the tables. In such cases the
- // routing need not be tenant aware.
- // Eg: SELECT 1 AS one FROM select_having_expression_test_test_having
HAVING 1 > 2;
- serverInstances = new ArrayList<>(enabledServerInstanceMap.values());
- } else {
- Set<String> servers = new HashSet<>();
- for (String tableName : tableNames) {
- TableType tableType =
TableNameBuilder.getTableTypeFromTableName(tableName);
- if (tableType == null) {
- Set<String> offlineTableServers =
_routingManager.getServingInstances(
-
TableNameBuilder.forType(TableType.OFFLINE).tableNameWithType(tableName));
- if (offlineTableServers != null) {
- servers.addAll(offlineTableServers);
- }
- Set<String> realtimeTableServers =
_routingManager.getServingInstances(
-
TableNameBuilder.forType(TableType.REALTIME).tableNameWithType(tableName));
- if (realtimeTableServers != null) {
- servers.addAll(realtimeTableServers);
- }
- } else {
- Set<String> tableServers =
_routingManager.getServingInstances(tableName);
- if (tableServers != null) {
- servers.addAll(tableServers);
- }
+ Set<String> servers = new HashSet<>();
+ for (String tableName : tableNames) {
+ TableType tableType =
TableNameBuilder.getTableTypeFromTableName(tableName);
+ if (tableType == null) {
+ Set<String> offlineTableServers = _routingManager.getServingInstances(
+
TableNameBuilder.forType(TableType.OFFLINE).tableNameWithType(tableName));
+ if (offlineTableServers != null) {
+ servers.addAll(offlineTableServers);
+ }
+ Set<String> realtimeTableServers = _routingManager.getServingInstances(
+
TableNameBuilder.forType(TableType.REALTIME).tableNameWithType(tableName));
+ if (realtimeTableServers != null) {
+ servers.addAll(realtimeTableServers);
}
- }
- if (servers.isEmpty()) {
- // fall back to use all enabled servers if no server is found for the
tables
- serverInstances = new ArrayList<>(enabledServerInstanceMap.values());
} else {
- serverInstances = new ArrayList<>(servers.size());
- for (String server : servers) {
- ServerInstance serverInstance = enabledServerInstanceMap.get(server);
- if (serverInstance != null) {
- serverInstances.add(serverInstance);
- }
+ Set<String> tableServers =
_routingManager.getServingInstances(tableName);
+ if (tableServers != null) {
+ servers.addAll(tableServers);
+ }
+ }
+ }
+ if (servers.isEmpty()) {
+ // Fall back to use all enabled servers if no server is found for the
tables.
+ // TODO: Revisit if we should throw an exception instead.
Review Comment:
In what scenarios would this occur?
--
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]
