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 2589860e1de Drop null-valued entries from cluster config snapshots
(#19594)
2589860e1de is described below
commit 2589860e1dea403d08cc9feacf4c0c90e6752ad7
Author: Xiaotian (Jackie) Jiang <[email protected]>
AuthorDate: Thu Sep 17 16:52:14 2026 -0700
Drop null-valued entries from cluster config snapshots (#19594)
---
.../config/DefaultClusterConfigChangeHandler.java | 2 +-
.../spi/config/provider/PinotClusterConfigProvider.java | 16 ++++++++++++++++
.../config/provider/PinotClusterConfigProviderTest.java | 17 ++++++++++++++++-
3 files changed, 33 insertions(+), 2 deletions(-)
diff --git
a/pinot-common/src/main/java/org/apache/pinot/common/config/DefaultClusterConfigChangeHandler.java
b/pinot-common/src/main/java/org/apache/pinot/common/config/DefaultClusterConfigChangeHandler.java
index cafdacb9328..d26d86f1308 100644
---
a/pinot-common/src/main/java/org/apache/pinot/common/config/DefaultClusterConfigChangeHandler.java
+++
b/pinot-common/src/main/java/org/apache/pinot/common/config/DefaultClusterConfigChangeHandler.java
@@ -43,7 +43,7 @@ public class DefaultClusterConfigChangeHandler implements
ClusterConfigChangeLis
@Override
public synchronized void onClusterConfigChange(ClusterConfig clusterConfig,
NotificationContext context) {
- Map<String, String> clusterConfigs =
Map.copyOf(clusterConfig.getRecord().getSimpleFields());
+ Map<String, String> clusterConfigs =
copyWithoutNullValues(clusterConfig.getRecord().getSimpleFields());
Set<String> changedConfigs = getChangedProperties(_clusterConfigs,
clusterConfigs);
LOGGER.info("Cluster configs changed: {}", changedConfigs);
_clusterConfigs = clusterConfigs;
diff --git
a/pinot-spi/src/main/java/org/apache/pinot/spi/config/provider/PinotClusterConfigProvider.java
b/pinot-spi/src/main/java/org/apache/pinot/spi/config/provider/PinotClusterConfigProvider.java
index 7209b0f36ce..2abf37aa45a 100644
---
a/pinot-spi/src/main/java/org/apache/pinot/spi/config/provider/PinotClusterConfigProvider.java
+++
b/pinot-spi/src/main/java/org/apache/pinot/spi/config/provider/PinotClusterConfigProvider.java
@@ -18,6 +18,8 @@
*/
package org.apache.pinot.spi.config.provider;
+import com.google.common.collect.Maps;
+import java.util.Collections;
import java.util.HashSet;
import java.util.Map;
import java.util.Set;
@@ -36,6 +38,20 @@ public interface PinotClusterConfigProvider {
/// @return returns 'true' if the registration was successful
boolean registerClusterConfigChangeListener(PinotClusterConfigChangeListener
clusterConfigChangeListener);
+ /// Copies the ZK cluster configs without the null-valued entries. The
cluster config update API stores a JSON
+ /// `null` as a null value, which carries no information for a `String`
config, so the key is treated as absent.
+ /// @param clusterConfigs map of ZK cluster configs
+ /// @return immutable copy of the cluster configs without the null-valued
entries
+ default Map<String, String> copyWithoutNullValues(Map<String, String>
clusterConfigs) {
+ Map<String, String> copy =
Maps.newHashMapWithExpectedSize(clusterConfigs.size());
+ for (Map.Entry<String, String> entry : clusterConfigs.entrySet()) {
+ if (entry.getValue() != null) {
+ copy.put(entry.getKey(), entry.getValue());
+ }
+ }
+ return Collections.unmodifiableMap(copy);
+ }
+
/// Calculates the set of keys that changed in ZK cluster configs between
the old and new
/// @param oldProperties map of previously cached ZK cluster configs
/// @param newProperties map of newly fetched ZK cluster configs
diff --git
a/pinot-spi/src/test/java/org/apache/pinot/spi/config/provider/PinotClusterConfigProviderTest.java
b/pinot-spi/src/test/java/org/apache/pinot/spi/config/provider/PinotClusterConfigProviderTest.java
index a977d85d037..b236a96d94d 100644
---
a/pinot-spi/src/test/java/org/apache/pinot/spi/config/provider/PinotClusterConfigProviderTest.java
+++
b/pinot-spi/src/test/java/org/apache/pinot/spi/config/provider/PinotClusterConfigProviderTest.java
@@ -26,7 +26,7 @@ import org.testng.annotations.Test;
import static org.assertj.core.api.Assertions.assertThat;
-/// Unit tests for DefaultClusterConfigChangeHandler focusing on the
getChangedProperties method.
+/// Unit tests for the default methods of [PinotClusterConfigProvider].
public class PinotClusterConfigProviderTest {
@Test
@@ -197,6 +197,21 @@ public class PinotClusterConfigProviderTest {
assertThat(changedProperties).isEmpty();
}
+ @Test
+ public void testCopyWithoutNullValues() {
+ // Given
+ PinotClusterConfigProviderMock handler = new
PinotClusterConfigProviderMock();
+ Map<String, String> clusterConfigs = new HashMap<>();
+ clusterConfigs.put("key1", "value1");
+ clusterConfigs.put("key2", null);
+
+ // When
+ Map<String, String> copy = handler.copyWithoutNullValues(clusterConfigs);
+
+ // Then
+ assertThat(copy).hasSize(1).containsEntry("key1", "value1");
+ }
+
public static class PinotClusterConfigProviderMock implements
PinotClusterConfigProvider {
@Override
public Map<String, String> getClusterConfigs() {
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
