From 86a82dd410328265a2aadd2282a79af1e13f6182 Mon Sep 17 00:00:00 2001 From: Chaitanya Deepthi <45308220+deepthi912@users.noreply.github.com> Date: Thu, 26 Dec 2024 18:27:36 -0800 Subject: [PATCH] Extend to add Default Server level configs for Dedup Tables (#14684) --- .../realtime/RealtimeTableDataManager.java | 3 ++- .../tests/DedupPreloadIntegrationTest.java | 6 ++++++ .../TableDedupMetadataManagerFactory.java | 19 ++++++++++++++++++- .../TableDedupMetadataManagerFactoryTest.java | 5 +++-- .../mutable/MutableSegmentDedupeTest.java | 2 +- .../helix/HelixInstanceDataManagerConfig.java | 9 +++++++++ .../instance/InstanceDataManagerConfig.java | 2 ++ .../pinot/spi/config/table/DedupConfig.java | 6 +++++- 8 files changed, 46 insertions(+), 6 deletions(-) diff --git a/pinot-core/src/main/java/org/apache/pinot/core/data/manager/realtime/RealtimeTableDataManager.java b/pinot-core/src/main/java/org/apache/pinot/core/data/manager/realtime/RealtimeTableDataManager.java index 2b4778d3904f..7cb1a7a5bd93 100644 --- a/pinot-core/src/main/java/org/apache/pinot/core/data/manager/realtime/RealtimeTableDataManager.java +++ b/pinot-core/src/main/java/org/apache/pinot/core/data/manager/realtime/RealtimeTableDataManager.java @@ -194,7 +194,8 @@ protected void doInit() { List 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/pinot-integration-tests/src/test/java/org/apache/pinot/integration/tests/DedupPreloadIntegrationTest.java b/pinot-integration-tests/src/test/java/org/apache/pinot/integration/tests/DedupPreloadIntegrationTest.java index c2589bb52011..ecba43245574 100644 --- a/pinot-integration-tests/src/test/java/org/apache/pinot/integration/tests/DedupPreloadIntegrationTest.java +++ b/pinot-integration-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/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/dedup/TableDedupMetadataManagerFactory.java b/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/dedup/TableDedupMetadataManagerFactory.java index 7f1aa2d42d0a..a94b4385a59a 100644 --- a/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/dedup/TableDedupMetadataManagerFactory.java +++ b/pinot-segment-local/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; @@ -34,15 +36,30 @@ private TableDedupMetadataManagerFactory() { } private static final Logger LOGGER = LoggerFactory.getLogger(TableDedupMetadataManagerFactory.class); + public static final String DEDUP_DEFAULT_METADATA_MANAGER_CLASS = "default.metadata.manager.class"; + 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/pinot-segment-local/src/test/java/org/apache/pinot/segment/local/dedup/TableDedupMetadataManagerFactoryTest.java b/pinot-segment-local/src/test/java/org/apache/pinot/segment/local/dedup/TableDedupMetadataManagerFactoryTest.java index f3247c822734..3f2fe600cf84 100644 --- a/pinot-segment-local/src/test/java/org/apache/pinot/segment/local/dedup/TableDedupMetadataManagerFactoryTest.java +++ b/pinot-segment-local/src/test/java/org/apache/pinot/segment/local/dedup/TableDedupMetadataManagerFactoryTest.java @@ -54,7 +54,7 @@ public void testEnablePreload() { when(tableDataManager.getTableDataDir()).thenReturn(new File("mytable")); when(tableDataManager.getSegmentPreloadExecutor()).thenReturn(null); TableDedupMetadataManager tableDedupMetadataManager = - TableDedupMetadataManagerFactory.create(tableConfig, schema, tableDataManager, null); + TableDedupMetadataManagerFactory.create(tableConfig, schema, tableDataManager, null, null); assertNotNull(tableDedupMetadataManager); assertFalse(tableDedupMetadataManager.isEnablePreload()); @@ -62,7 +62,8 @@ public void testEnablePreload() { tableDataManager = mock(TableDataManager.class); when(tableDataManager.getTableDataDir()).thenReturn(new File("mytable")); when(tableDataManager.getSegmentPreloadExecutor()).thenReturn(mock(ExecutorService.class)); - tableDedupMetadataManager = TableDedupMetadataManagerFactory.create(tableConfig, schema, tableDataManager, null); + tableDedupMetadataManager = TableDedupMetadataManagerFactory.create(tableConfig, schema, tableDataManager, null, + null); assertNotNull(tableDedupMetadataManager); } } diff --git a/pinot-segment-local/src/test/java/org/apache/pinot/segment/local/indexsegment/mutable/MutableSegmentDedupeTest.java b/pinot-segment-local/src/test/java/org/apache/pinot/segment/local/indexsegment/mutable/MutableSegmentDedupeTest.java index b4544979e3cc..bb21b7b11cea 100644 --- a/pinot-segment-local/src/test/java/org/apache/pinot/segment/local/indexsegment/mutable/MutableSegmentDedupeTest.java +++ b/pinot-segment-local/src/test/java/org/apache/pinot/segment/local/indexsegment/mutable/MutableSegmentDedupeTest.java @@ -98,7 +98,7 @@ private static TableDedupMetadataManager getTableDedupMetadataManager(Schema sch TableDataManager tableDataManager = Mockito.mock(TableDataManager.class); Mockito.when(tableDataManager.getTableDataDir()).thenReturn(TEMP_DIR); return TableDedupMetadataManagerFactory.create(tableConfig, schema, tableDataManager, - Mockito.mock(ServerMetrics.class)); + Mockito.mock(ServerMetrics.class), null); } public List> loadJsonFile(String filePath) diff --git a/pinot-server/src/main/java/org/apache/pinot/server/starter/helix/HelixInstanceDataManagerConfig.java b/pinot-server/src/main/java/org/apache/pinot/server/starter/helix/HelixInstanceDataManagerConfig.java index aade26f339af..b666d990f09a 100644 --- a/pinot-server/src/main/java/org/apache/pinot/server/starter/helix/HelixInstanceDataManagerConfig.java +++ b/pinot-server/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 dedup 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> _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 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 index 52e9b6f9f23c..64d8de88b279 100644 --- 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> 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 index dfc8151e3589..b1e6caec3023 100644 --- 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 void setEnablePreload(boolean enablePreload) { + _enablePreload = enablePreload; + } }