Jackie-Jiang opened a new issue, #19579: URL: https://github.com/apache/pinot/issues/19579
## Problem Pinot has two precedence rules for a config that is set in both an instance config and the ZK cluster config, depending on how the config is read: - Configs read once at startup go through `ServiceStartableUtils.applyClusterConfig`, which folds the cluster config into the instance config with `addConfigIfNotExists`: **the instance config wins**. - Configs applied live through `DefaultClusterConfigChangeHandler` / `PinotClusterConfigChangeListener` see the raw cluster config: **the cluster config wins**, because `registerClusterConfigChangeListener` immediately fires `onChange` with the current cluster config and every later change is applied as-is. There is no way to override a dynamic cluster config on one instance. The folding also hides where a value came from. A listener that falls back to the instance config when a key is removed from the cluster config (`ServerRoutingStatsManager` until #19578) reads the cluster value that was folded in at startup, i.e. the very value being removed, and keeps it until a restart. Listeners that avoid the trap by never reading the instance config (`ContinuousJfrStarter`, `ConsumingSegmentConsistencyModeListener`, `QueryOptionConfigListener` in #19570) ignore instance configs entirely instead. ## Proposal Make the precedence uniform, instance config > cluster config > default, and implement it once in `DefaultClusterConfigChangeHandler` rather than in every listener: - Each starter (broker, controller, server, minion) snapshots the instance config in `init()` **before** `applyClusterConfig` runs, the only moment file vs. cluster provenance is still known, and hands it to the handler. - `process()` overlays the snapshot on the cluster configs and diffs that *effective* view. A key set in the instance config is constant in the effective view, so it never shows up in `changedConfigs`: cluster changes and removals of it are invisible on that instance. Unpinned keys behave exactly as today. - Registration fires `onChange(effective.keySet(), effective)`, so an instance config value reaches listeners that only consult the cluster config, without touching them. - `PinotClusterConfigProvider.getClusterConfigs()` returns the effective view; its Javadoc should say so. No per-listener changes; the removal fallback question disappears (removal of an unpinned key restores the default, a pinned key never changes). This is a behavior change for the both-set case of existing dynamic configs, where the cluster value currently wins, so it should carry `release-notes`. ## Ordering Builds on top of #19570 (adds `QueryOptionConfigListener`, whose Javadoc currently says the instance config is not consulted and should be updated) and #19578 (removes the folded-config fallback in `ServerRoutingStatsManager`). -- 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]
