Add settings to pick up default settings from server configs

apache · Dec 18, 2024 · 12ca31f · 12ca31f
1 parent 7cf7da4
commit 12ca31f
Show file tree

Hide file tree

Showing 6 changed files with 39 additions and 3 deletions.
diff --git a/...e/src/main/java/org/apache/pinot/core/data/manager/realtime/RealtimeTableDataManager.java b/...e/src/main/java/org/apache/pinot/core/data/manager/realtime/RealtimeTableDataManager.java
@@ -194,7 +194,8 @@ protected void doInit() {
       List<String> primaryKeyColumns = schema.getPrimaryKeyColumns();
       Preconditions.checkState(!CollectionUtils.isEmpty(primaryKeyColumns),
           "Primary key columns must be configured for dedup");
-      _tableDedupMetadataManager = TableDedupMetadataManagerFactory.create(_tableConfig, schema, this, _serverMetrics);
+      _tableDedupMetadataManager = TableDedupMetadataManagerFactory.create(_tableConfig, schema, this, _serverMetrics,
+          _instanceDataManagerConfig.getDedupConfig());
     }
 
     UpsertConfig upsertConfig = _tableConfig.getUpsertConfig();

diff --git a/...n-tests/src/test/java/org/apache/pinot/integration/tests/DedupPreloadIntegrationTest.java b/...n-tests/src/test/java/org/apache/pinot/integration/tests/DedupPreloadIntegrationTest.java
@@ -18,12 +18,15 @@
  */
 package org.apache.pinot.integration.tests;
 
+import com.google.common.base.Joiner;
 import java.io.File;
 import java.io.IOException;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 import org.apache.commons.io.FileUtils;
+import org.apache.pinot.segment.local.dedup.TableDedupMetadataManagerFactory;
+import org.apache.pinot.server.starter.helix.HelixInstanceDataManagerConfig;
 import org.apache.pinot.spi.config.table.ColumnPartitionConfig;
 import org.apache.pinot.spi.config.table.DedupConfig;
 import org.apache.pinot.spi.config.table.HashFunction;
@@ -76,6 +79,9 @@ public void setUp()
   protected void overrideServerConf(PinotConfiguration serverConf) {
     serverConf.setProperty(CommonConstants.Server.INSTANCE_DATA_MANAGER_CONFIG_PREFIX + ".max.segment.preload.threads",
         "1");
+    serverConf.setProperty(Joiner.on(".").join(CommonConstants.Server.INSTANCE_DATA_MANAGER_CONFIG_PREFIX,
+        HelixInstanceDataManagerConfig.DEDUP_CONFIG_PREFIX,
+        TableDedupMetadataManagerFactory.DEDUP_DEFAULT_ENABLE_PRELOAD), "true");
   }
 
   @AfterClass

diff --git a/.../src/main/java/org/apache/pinot/segment/local/dedup/TableDedupMetadataManagerFactory.java b/.../src/main/java/org/apache/pinot/segment/local/dedup/TableDedupMetadataManagerFactory.java
@@ -19,12 +19,14 @@
 package org.apache.pinot.segment.local.dedup;
 
 import com.google.common.base.Preconditions;
+import javax.annotation.Nullable;
 import org.apache.commons.lang3.StringUtils;
 import org.apache.pinot.common.metrics.ServerMetrics;
 import org.apache.pinot.segment.local.data.manager.TableDataManager;
 import org.apache.pinot.spi.config.table.DedupConfig;
 import org.apache.pinot.spi.config.table.TableConfig;
 import org.apache.pinot.spi.data.Schema;
+import org.apache.pinot.spi.env.PinotConfiguration;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -38,13 +40,25 @@ private TableDedupMetadataManagerFactory() {
   public static final String DEDUP_DEFAULT_ENABLE_PRELOAD = "default.enable.preload";
 
   public static TableDedupMetadataManager create(TableConfig tableConfig, Schema schema,
-      TableDataManager tableDataManager, ServerMetrics serverMetrics) {
+      TableDataManager tableDataManager, ServerMetrics serverMetrics, @Nullable PinotConfiguration instanceDedupConfig) {
     String tableNameWithType = tableConfig.getTableName();
     DedupConfig dedupConfig = tableConfig.getDedupConfig();
     Preconditions.checkArgument(dedupConfig != null, "Must provide dedup config for table: %s", tableNameWithType);
 
     TableDedupMetadataManager metadataManager;
     String metadataManagerClass = dedupConfig.getMetadataManagerClass();
+
+    if (instanceDedupConfig != null) {
+      if (metadataManagerClass == null) {
+        metadataManagerClass = instanceDedupConfig.getProperty(DEDUP_DEFAULT_METADATA_MANAGER_CLASS);
+      }
+
+      // Server level config honoured only when table level config is not set to true
+      if (!dedupConfig.isEnablePreload()) {
+        dedupConfig.setEnablePreload(
+            Boolean.parseBoolean(instanceDedupConfig.getProperty(DEDUP_DEFAULT_ENABLE_PRELOAD, "false")));
+      }
+    }
     if (StringUtils.isNotEmpty(metadataManagerClass)) {
       LOGGER.info("Creating TableDedupMetadataManager with class: {} for table: {}", metadataManagerClass,
           tableNameWithType);

diff --git a/...r/src/main/java/org/apache/pinot/server/starter/helix/HelixInstanceDataManagerConfig.java b/...r/src/main/java/org/apache/pinot/server/starter/helix/HelixInstanceDataManagerConfig.java
@@ -49,6 +49,8 @@ public class HelixInstanceDataManagerConfig implements InstanceDataManagerConfig
   public static final String SEGMENT_DIRECTORY_LOADER = "segment.directory.loader";
   // Prefix for upsert config
   public static final String UPSERT_CONFIG_PREFIX = "upsert";
+  // Prefix for upsert config
+  public static final String DEDUP_CONFIG_PREFIX = "dedup";
   // Prefix for auth config
   public static final String AUTH_CONFIG_PREFIX = "auth";
   // Prefix for tier configs
@@ -118,6 +120,7 @@ public class HelixInstanceDataManagerConfig implements InstanceDataManagerConfig
 
   private final PinotConfiguration _serverConfig;
   private final PinotConfiguration _upsertConfig;
+  private final PinotConfiguration _dedupConfig;
   private final PinotConfiguration _authConfig;
   private final Map<String, Map<String, String>> _tierConfigs;
 
@@ -133,6 +136,7 @@ public HelixInstanceDataManagerConfig(PinotConfiguration serverConfig)
 
     _authConfig = serverConfig.subset(AUTH_CONFIG_PREFIX);
     _upsertConfig = serverConfig.subset(UPSERT_CONFIG_PREFIX);
+    _dedupConfig = serverConfig.subset(DEDUP_CONFIG_PREFIX);
 
     PinotConfiguration tierConfigs = getConfig().subset(TIER_CONFIGS_PREFIX);
     List<String> tierNames = tierConfigs.getProperty(TIER_NAMES, Collections.emptyList());
@@ -289,6 +293,11 @@ public PinotConfiguration getUpsertConfig() {
     return _upsertConfig;
   }
 
+  @Override
+  public PinotConfiguration getDedupConfig() {
+    return _dedupConfig;
+  }
+
   @Override
   public PinotConfiguration getAuthConfig() {
     return _authConfig;

diff --git a/pinot-spi/src/main/java/org/apache/pinot/spi/config/instance/InstanceDataManagerConfig.java b/pinot-spi/src/main/java/org/apache/pinot/spi/config/instance/InstanceDataManagerConfig.java
@@ -73,6 +73,8 @@ public interface InstanceDataManagerConfig {
 
   PinotConfiguration getUpsertConfig();
 
+  PinotConfiguration getDedupConfig();
+
   PinotConfiguration getAuthConfig();
 
   Map<String, Map<String, String>> getTierConfigs();

diff --git a/pinot-spi/src/main/java/org/apache/pinot/spi/config/table/DedupConfig.java b/pinot-spi/src/main/java/org/apache/pinot/spi/config/table/DedupConfig.java
@@ -45,7 +45,7 @@ public class DedupConfig extends BaseJsonConfig {
   private final String _dedupTimeColumn;
 
   @JsonPropertyDescription("Whether to preload segments for fast dedup metadata recovery")
-  private final boolean _enablePreload;
+  private boolean _enablePreload;
 
   public DedupConfig(@JsonProperty(value = "dedupEnabled", required = true) boolean dedupEnabled,
       @JsonProperty(value = "hashFunction") HashFunction hashFunction) {
@@ -96,4 +96,8 @@ public String getDedupTimeColumn() {
   public boolean isEnablePreload() {
     return _enablePreload;
   }
+
+  public boolean setEnablePreload(boolean enablePreload) {
+    return _enablePreload = enablePreload;
+  }
 }