This is an automated email from the ASF dual-hosted git repository.
Jackie-Jiang pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/pinot.git
The following commit(s) were added to refs/heads/master by this push:
new c1a26e91c1e Restore the default when an adaptive routing metric export
cluster config is removed (#19578)
c1a26e91c1e is described below
commit c1a26e91c1e5f169de5ba8c90f1a904a1f43d30c
Author: Xiaotian (Jackie) Jiang <[email protected]>
AuthorDate: Wed Sep 16 16:45:14 2026 -0700
Restore the default when an adaptive routing metric export cluster config
is removed (#19578)
---
.../routing/stats/ServerRoutingStatsManager.java | 14 +++-----
.../stats/ServerRoutingStatsManagerTest.java | 41 ++++++++++++++++++++--
2 files changed, 43 insertions(+), 12 deletions(-)
diff --git
a/pinot-core/src/main/java/org/apache/pinot/core/transport/server/routing/stats/ServerRoutingStatsManager.java
b/pinot-core/src/main/java/org/apache/pinot/core/transport/server/routing/stats/ServerRoutingStatsManager.java
index 6b80990de63..c939c99829b 100644
---
a/pinot-core/src/main/java/org/apache/pinot/core/transport/server/routing/stats/ServerRoutingStatsManager.java
+++
b/pinot-core/src/main/java/org/apache/pinot/core/transport/server/routing/stats/ServerRoutingStatsManager.java
@@ -133,14 +133,11 @@ public class ServerRoutingStatsManager implements
PinotClusterConfigChangeListen
@Override
public void onChange(Set<String> changedConfigs, Map<String, String>
clusterConfigs) {
if
(changedConfigs.contains(AdaptiveServerSelector.CONFIG_OF_ENABLE_STATS_METRIC_EXPORT))
{
+ // A removed key restores the default. The broker config is no fallback:
the cluster config present at startup
+ // was folded into it, so it may still hold the value being removed
String value =
clusterConfigs.get(AdaptiveServerSelector.CONFIG_OF_ENABLE_STATS_METRIC_EXPORT);
- if (value != null) {
- _enableStatsMetricExport = Boolean.parseBoolean(value);
- } else {
- // Key was removed from cluster config — fall back to the static
broker config value.
- _enableStatsMetricExport =
_config.getProperty(AdaptiveServerSelector.CONFIG_OF_ENABLE_STATS_METRIC_EXPORT,
- AdaptiveServerSelector.DEFAULT_ENABLE_STATS_METRIC_EXPORT);
- }
+ _enableStatsMetricExport =
+ value != null ? Boolean.parseBoolean(value) :
AdaptiveServerSelector.DEFAULT_ENABLE_STATS_METRIC_EXPORT;
LOGGER.info("Updated enableStatsMetricExport to {} from cluster
config.", _enableStatsMetricExport);
if (!_enableStatsMetricExport) {
removeAllServerStatsGauges();
@@ -163,8 +160,7 @@ public class ServerRoutingStatsManager implements
PinotClusterConfigChangeListen
return;
}
} else {
- newIntervalMs =
_config.getProperty(AdaptiveServerSelector.CONFIG_OF_STATS_METRIC_EXPORT_INTERVAL_MS,
- AdaptiveServerSelector.DEFAULT_STATS_METRIC_EXPORT_INTERVAL_MS);
+ newIntervalMs =
AdaptiveServerSelector.DEFAULT_STATS_METRIC_EXPORT_INTERVAL_MS;
}
if (newIntervalMs != _statsMetricExportIntervalMs) {
_statsMetricExportIntervalMs = newIntervalMs;
diff --git
a/pinot-core/src/test/java/org/apache/pinot/core/transport/server/routing/stats/ServerRoutingStatsManagerTest.java
b/pinot-core/src/test/java/org/apache/pinot/core/transport/server/routing/stats/ServerRoutingStatsManagerTest.java
index 99a947b980a..b1b282514b5 100644
---
a/pinot-core/src/test/java/org/apache/pinot/core/transport/server/routing/stats/ServerRoutingStatsManagerTest.java
+++
b/pinot-core/src/test/java/org/apache/pinot/core/transport/server/routing/stats/ServerRoutingStatsManagerTest.java
@@ -501,6 +501,40 @@ public class ServerRoutingStatsManagerTest {
assertNull(_brokerMetrics.getGaugeValue(numInFlightKey));
}
+ @Test
+ public void testStatsMetricExportKeyRemovalRestoresDefault() throws
InterruptedException {
+ Map<String, Object> properties = new HashMap<>();
+
properties.put(CommonConstants.Broker.AdaptiveServerSelector.CONFIG_OF_ENABLE_STATS_COLLECTION,
true);
+
properties.put(CommonConstants.Broker.AdaptiveServerSelector.CONFIG_OF_EWMA_ALPHA,
1.0);
+
properties.put(CommonConstants.Broker.AdaptiveServerSelector.CONFIG_OF_AUTODECAY_WINDOW_MS,
-1);
+
properties.put(CommonConstants.Broker.AdaptiveServerSelector.CONFIG_OF_WARMUP_DURATION_MS,
0);
+
properties.put(CommonConstants.Broker.AdaptiveServerSelector.CONFIG_OF_AVG_INITIALIZATION_VAL,
0.0);
+
properties.put(CommonConstants.Broker.AdaptiveServerSelector.CONFIG_OF_HYBRID_SCORE_EXPONENT,
3);
+ // Metric export is enabled in the broker config, which also holds the
cluster config folded in at startup.
+
properties.put(CommonConstants.Broker.AdaptiveServerSelector.CONFIG_OF_ENABLE_STATS_METRIC_EXPORT,
true);
+
properties.put(CommonConstants.Broker.AdaptiveServerSelector.CONFIG_OF_STATS_METRIC_EXPORT_INTERVAL_MS,
50L);
+ ServerRoutingStatsManager manager = new ServerRoutingStatsManager(new
PinotConfiguration(properties),
+ _brokerMetrics);
+ manager.init();
+
+ int requestId = 0;
+ manager.recordStatsForQuerySubmission(requestId++, "keyRemovalServer");
+ waitForStatsUpdate(manager, requestId);
+ manager.recordStatsUponResponseArrival(requestId++, "keyRemovalServer",
100);
+ waitForStatsUpdate(manager, requestId);
+
+ String numInFlightKey =
BrokerGauge.ADAPTIVE_SERVER_NUM_IN_FLIGHT_REQUESTS.getGaugeName()
+ + ".server.keyRemovalServer";
+ TestUtils.waitForCondition(aVoid ->
_brokerMetrics.getGaugeValue(numInFlightKey) != null,
+ 50L, 5000, "Timed out waiting for metrics");
+
+ // Removing the key from the cluster config restores the default
(disabled) rather than the broker config value.
+ manager.onChange(
+
Set.of(CommonConstants.Broker.AdaptiveServerSelector.CONFIG_OF_ENABLE_STATS_METRIC_EXPORT),
Map.of());
+
+ assertNull(_brokerMetrics.getGaugeValue(numInFlightKey));
+ }
+
@Test
public void testStatsMetricExportIntervalDynamicUpdate() throws
InterruptedException {
Map<String, Object> properties = new HashMap<>();
@@ -577,12 +611,13 @@ public class ServerRoutingStatsManagerTest {
assertEquals(manager.getStatsMetricExportIntervalMs(), intervalBefore,
"Interval must not change on negative config value");
- // Key removed from cluster config — must fall back to the static broker
config value (100000L).
+ // Key removed from cluster config: restores the default rather than the
broker config value.
manager.onChange(
Set.of(CommonConstants.Broker.AdaptiveServerSelector.CONFIG_OF_STATS_METRIC_EXPORT_INTERVAL_MS),
Map.of());
- assertEquals(manager.getStatsMetricExportIntervalMs(), 100000L,
- "Interval must revert to static config when cluster key is removed");
+ assertEquals(manager.getStatsMetricExportIntervalMs(),
+
CommonConstants.Broker.AdaptiveServerSelector.DEFAULT_STATS_METRIC_EXPORT_INTERVAL_MS,
+ "Interval must revert to the default when cluster key is removed");
manager.shutDown();
}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
