Jackie-Jiang commented on code in PR #16083:
URL: https://github.com/apache/pinot/pull/16083#discussion_r2143245694
##########
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:
I feel we might never reach here unless some super rare corner case where
server is available when assigning worker for the leaf stages, then
disabled/shut down just after that. Left a TODO instead of throwing exception
here since it is out of the scope of this PR. We can revisit it separately
--
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
