yashmayya opened a new pull request, #19642:
URL: https://github.com/apache/pinot/pull/19642

   ## The bug
   
   If `pinot.broker.mse.planner.disabled.rules` is set in the cluster config to 
more than one rule, for example `SortJoinCopy,AggregateUnionAggregate`, the 
broker disables none of them.
   
   `ServiceStartableUtils.applyClusterConfig` copies cluster configs into the 
broker config with `PinotConfiguration.setProperty`. This method keeps a 
comma-separated value as one string. As a result, `getProperty(key, List)` 
returns one element, `"SortJoinCopy,AggregateUnionAggregate"`, and no rule has 
that name. The value also replaces `DEFAULT_DISABLED_RULES`, so the rules that 
are off by default (for example `SortProjectTranspose`) run too.
   
   The same value in the broker config file works, because a config that is 
built from a map or a file splits on commas.
   
   ## The fix
   
   `MultiStageBrokerRequestHandler` now splits each value of this config on 
commas, trims it, and drops empty items. If the key is not set, the broker 
still uses `DEFAULT_DISABLED_RULES`. An empty value still disables no rules.
   
   ## Testing
   
   - New tests in `MultiStageBrokerRequestHandlerTest` cover the cluster config 
path (`setProperty`, the same call that `applyClusterConfig` makes), the broker 
config, a list value, and unset and empty values. The cluster config test fails 
with the old parsing logic.
   - A one-off check through a local ZK and the real `applyClusterConfig` gave 
1 rule before this change and 2 rules after it.
   - `spotless`, `checkstyle` and `license` are clean.
   
   ## Upgrade note
   
   If a cluster sets more than one rule in this cluster config, those rules 
become disabled after the upgrade. Query plans can change.
   
   ## Notes
   
   Other top-level list configs that are read with `getProperty(key, List)` 
have the same problem. This PR fixes only this config, to keep it small.
   
   ## Labels
   
   `bug`, `multi-stage`
   


-- 
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]

Reply via email to