somandal commented on code in PR #16964:
URL: https://github.com/apache/pinot/pull/16964#discussion_r2408978081
##########
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/PinotHelixResourceManager.java:
##########
@@ -3562,18 +3562,13 @@ public List<TableConfig>
getTableConfigsForSchema(String schemaName) {
public List<String> getServerInstancesForTable(String tableName, TableType
tableType) {
TableConfig tableConfig = getTableConfig(tableName, tableType);
Preconditions.checkNotNull(tableConfig);
- TenantConfig tenantConfig = tableConfig.getTenantConfig();
Set<String> serverInstances = new HashSet<>();
- List<InstanceConfig> instanceConfigs =
HelixHelper.getInstanceConfigs(_helixZkManager);
- if (tableType == TableType.OFFLINE) {
- serverInstances.addAll(
- HelixHelper.getInstancesWithTag(instanceConfigs,
TagNameUtils.extractOfflineServerTag(tenantConfig)));
- } else if (TableType.REALTIME.equals(tableType)) {
- serverInstances.addAll(
- HelixHelper.getInstancesWithTag(instanceConfigs,
TagNameUtils.extractConsumingServerTag(tenantConfig)));
- serverInstances.addAll(
- HelixHelper.getInstancesWithTag(instanceConfigs,
TagNameUtils.extractCompletedServerTag(tenantConfig)));
+ IdealState idealState = getTableIdealState(tableConfig.getTableName());
Review Comment:
I'm fine with IS. my point was mainly for consistency, since we had a lot of
discussion around this for needsReload, and the races here are similar, but
probably less important to worry about
It would be good to have a clear guideline about when it's good to use IS
vs. EV though
--
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]
