This is an automated email from the ASF dual-hosted git repository.
KKcorps 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 481d4768b72 Revert "Add per-table hybrid retention strategy with
cluster-level fallback (…" (#20101)
481d4768b72 is described below
commit 481d4768b72efec495228c0ce0114c81f9e47f27
Author: Manak Bisht <[email protected]>
AuthorDate: Fri Oct 30 14:02:33 2026 +0530
Revert "Add per-table hybrid retention strategy with cluster-level fallback
(…" (#20101)
This reverts commit 20cac7096447a33b809bf5a317b082a09dfbdd51.
---
.../utils/config/TableConfigSerDeUtilsTest.java | 28 --------------
.../helix/core/retention/RetentionManager.java | 15 +-------
.../helix/core/retention/RetentionManagerTest.java | 44 ----------------------
.../SegmentsValidationAndRetentionConfig.java | 11 ------
4 files changed, 2 insertions(+), 96 deletions(-)
diff --git
a/pinot-common/src/test/java/org/apache/pinot/common/utils/config/TableConfigSerDeUtilsTest.java
b/pinot-common/src/test/java/org/apache/pinot/common/utils/config/TableConfigSerDeUtilsTest.java
index 5118b4f5936..4aaa37e4363 100644
---
a/pinot-common/src/test/java/org/apache/pinot/common/utils/config/TableConfigSerDeUtilsTest.java
+++
b/pinot-common/src/test/java/org/apache/pinot/common/utils/config/TableConfigSerDeUtilsTest.java
@@ -374,34 +374,6 @@ public class TableConfigSerDeUtilsTest {
assertEquals(tunerConfigToCompare.getName(), name);
assertEquals(tunerConfigToCompare.getTunerProperties(), props);
}
- {
- // With hybrid table retention strategy
- for (Boolean enableHybridTableRetentionStrategy : new Boolean[]{true,
false}) {
- TableConfig tableConfig = tableConfigBuilder.build();
-
tableConfig.getValidationConfig().setEnableHybridTableRetentionStrategy(enableHybridTableRetentionStrategy);
-
- // Serialize then de-serialize
- TableConfig tableConfigToCompare =
JsonUtils.stringToObject(tableConfig.toJsonString(), TableConfig.class);
- assertEquals(tableConfigToCompare, tableConfig);
-
assertEquals(tableConfigToCompare.getValidationConfig().getEnableHybridTableRetentionStrategy(),
- enableHybridTableRetentionStrategy);
-
- tableConfigToCompare =
TableConfigSerDeUtils.fromZNRecord(TableConfigSerDeUtils.toZNRecord(tableConfig));
- assertEquals(tableConfigToCompare, tableConfig);
-
assertEquals(tableConfigToCompare.getValidationConfig().getEnableHybridTableRetentionStrategy(),
- enableHybridTableRetentionStrategy);
- }
-
- // Unset by default
- TableConfig tableConfig = tableConfigBuilder.build();
-
assertNull(tableConfig.getValidationConfig().getEnableHybridTableRetentionStrategy());
- ObjectNode validationConfigJson = (ObjectNode)
tableConfig.toJsonNode().get(TableConfig.VALIDATION_CONFIG_KEY);
-
assertFalse(validationConfigJson.has("enableHybridTableRetentionStrategy"));
-
- // Configs without this field deserialize to null
- TableConfig tableConfigToCompare =
JsonUtils.stringToObject(tableConfig.toJsonString(), TableConfig.class);
-
assertNull(tableConfigToCompare.getValidationConfig().getEnableHybridTableRetentionStrategy());
- }
}
private void checkSegmentsValidationAndRetentionConfig(TableConfig
tableConfig) {
diff --git
a/pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/retention/RetentionManager.java
b/pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/retention/RetentionManager.java
index c4a7c490b9e..e2a65a92098 100644
---
a/pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/retention/RetentionManager.java
+++
b/pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/retention/RetentionManager.java
@@ -28,7 +28,6 @@ import java.util.HashMap;
import java.util.HashSet;
import java.util.List;
import java.util.Map;
-import java.util.Objects;
import java.util.Set;
import java.util.concurrent.TimeUnit;
import java.util.stream.Collectors;
@@ -355,22 +354,13 @@ public class RetentionManager extends
ControllerPeriodicTask<Void> {
// Scan all segment ZK metadata and purge segments if necessary
if (TableNameBuilder.isOfflineTableResource(tableNameWithType)) {
- if (validationConfig.getEnableHybridTableRetentionStrategy() != null) {
- LOGGER.warn(
- "enableHybridTableRetentionStrategy is only applicable to realtime
tables,"
- + "ignoring it for offline table: {}",
- tableNameWithType);
- }
manageRetentionForOfflineTable(tableNameWithType, retentionStrategy,
untrackedSegmentsDeletionBatchSize,
untrackedSegmentsRetentionStrategy);
} else {
String rawTableName =
TableNameBuilder.extractRawTableName(tableNameWithType);
TableConfig offlineTableConfig =
_pinotHelixResourceManager.getOfflineTableConfig(rawTableName);
// hybrid table check should be performed before the realtime table
check.
- boolean hybridTableRetentionStrategyEnabled =
-
Objects.requireNonNullElse(validationConfig.getEnableHybridTableRetentionStrategy(),
- _isHybridTableRetentionStrategyEnabled);
- if (hybridTableRetentionStrategyEnabled && offlineTableConfig != null) {
+ if (_isHybridTableRetentionStrategyEnabled && offlineTableConfig !=
null) {
// TODO: handle the orphan segment deletion for hybrid table
manageRetentionForHybridTable(tableConfig, offlineTableConfig);
} else {
@@ -402,8 +392,7 @@ public class RetentionManager extends
ControllerPeriodicTask<Void> {
}
}
- @VisibleForTesting
- void manageRetentionForRealtimeTable(String realtimeTableName,
RetentionStrategy retentionStrategy,
+ private void manageRetentionForRealtimeTable(String realtimeTableName,
RetentionStrategy retentionStrategy,
int untrackedSegmentsDeletionBatchSize, RetentionStrategy
untrackedSegmentsRetentionStrategy) {
List<SegmentZKMetadata> segmentZKMetadataList =
_pinotHelixResourceManager.getSegmentsZKMetadata(realtimeTableName);
diff --git
a/pinot-controller/src/test/java/org/apache/pinot/controller/helix/core/retention/RetentionManagerTest.java
b/pinot-controller/src/test/java/org/apache/pinot/controller/helix/core/retention/RetentionManagerTest.java
index 101969978a7..a442d598587 100644
---
a/pinot-controller/src/test/java/org/apache/pinot/controller/helix/core/retention/RetentionManagerTest.java
+++
b/pinot-controller/src/test/java/org/apache/pinot/controller/helix/core/retention/RetentionManagerTest.java
@@ -74,7 +74,6 @@ import org.apache.pinot.spi.utils.builder.TableNameBuilder;
import org.apache.zookeeper.data.Stat;
import org.testng.annotations.AfterMethod;
import org.testng.annotations.BeforeMethod;
-import org.testng.annotations.DataProvider;
import org.testng.annotations.Test;
import static
org.apache.pinot.controller.helix.core.retention.RetentionManager.DEFAULT_UNTRACKED_SEGMENTS_DELETION_BATCH_SIZE;
@@ -516,49 +515,6 @@ public class RetentionManagerTest {
verify(mockPinotHelixResourceManager,
times(1)).deleteSegments(eq(realtimeTableName), anyList());
}
- @DataProvider(name = "hybridTableRetentionStrategyOverrides")
- public Boolean[][] hybridTableRetentionStrategyOverrides() {
- // tableLevel (null falls back to clusterLevel), clusterLevel,
expectHybridPath
- return new Boolean[][]{
- {null, false, false},
- {null, true, true},
- {true, false, true},
- {false, true, false},
- };
- }
-
- @Test(dataProvider = "hybridTableRetentionStrategyOverrides")
- public void
testTableLevelHybridRetentionStrategyOverridesClusterConfig(Boolean tableLevel,
boolean clusterLevel,
- boolean expectHybridPath) {
- final int replicaCount = 1;
- TableConfig realtimeTableConfig = createRealtimeTableConfig1(replicaCount);
-
realtimeTableConfig.getValidationConfig().setEnableHybridTableRetentionStrategy(tableLevel);
-
- PinotHelixResourceManager resourceManager =
mock(PinotHelixResourceManager.class);
- when(resourceManager.getHelixAdmin()).thenReturn(mock(HelixAdmin.class));
- setupSizeRetentionOfflineCounterpart(resourceManager);
-
- ControllerConf conf = new ControllerConf();
- conf.setProperty(ControllerConf.ENABLE_HYBRID_TABLE_RETENTION_STRATEGY,
clusterLevel);
-
- RetentionManager retentionManager =
spy(createSizeRetentionManager(realtimeTableConfig, List.of(), resourceManager,
- mock(ControllerMetrics.class), conf, mock(BrokerServiceHelper.class)));
-
- // Only the retention strategy selection is under test, so stub out both
branches
- doNothing().when(retentionManager).manageRetentionForHybridTable(any(),
any());
- doNothing().when(retentionManager).manageRetentionForRealtimeTable(any(),
any(), anyInt(), any());
-
- retentionManager.processTable(REALTIME_TABLE_NAME);
-
- if (expectHybridPath) {
- verify(retentionManager).manageRetentionForHybridTable(any(), any());
- verify(retentionManager, never()).manageRetentionForRealtimeTable(any(),
any(), anyInt(), any());
- } else {
- verify(retentionManager, never()).manageRetentionForHybridTable(any(),
any());
- verify(retentionManager).manageRetentionForRealtimeTable(any(), any(),
anyInt(), any());
- }
- }
-
private TableConfig createOfflineTableConfig() {
return new
TableConfigBuilder(TableType.OFFLINE).setTableName(TEST_TABLE_NAME).setRetentionTimeUnit("DAYS")
.setRetentionTimeValue("365").setNumReplicas(2).build();
diff --git
a/pinot-spi/src/main/java/org/apache/pinot/spi/config/table/SegmentsValidationAndRetentionConfig.java
b/pinot-spi/src/main/java/org/apache/pinot/spi/config/table/SegmentsValidationAndRetentionConfig.java
index 4a165cf50e3..ae12ddae980 100644
---
a/pinot-spi/src/main/java/org/apache/pinot/spi/config/table/SegmentsValidationAndRetentionConfig.java
+++
b/pinot-spi/src/main/java/org/apache/pinot/spi/config/table/SegmentsValidationAndRetentionConfig.java
@@ -19,7 +19,6 @@
package org.apache.pinot.spi.config.table;
import java.util.concurrent.TimeUnit;
-import javax.annotation.Nullable;
import org.apache.pinot.spi.config.BaseJsonConfig;
import org.apache.pinot.spi.utils.TimeUtils;
@@ -32,7 +31,6 @@ public class SegmentsValidationAndRetentionConfig extends
BaseJsonConfig {
private String _deletedSegmentsRetentionPeriod;
private String _replacedSegmentsRetentionPeriod;
private String _lineageEntryCleanupRetentionPeriod;
- private Boolean _enableHybridTableRetentionStrategy;
@Deprecated
private String _segmentPushFrequency; // DO NOT REMOVE, this is used in
internal segment generation management
@Deprecated
@@ -186,15 +184,6 @@ public class SegmentsValidationAndRetentionConfig extends
BaseJsonConfig {
_lineageEntryCleanupRetentionPeriod = lineageEntryCleanupRetentionPeriod;
}
- @Nullable
- public Boolean getEnableHybridTableRetentionStrategy() {
- return _enableHybridTableRetentionStrategy;
- }
-
- public void setEnableHybridTableRetentionStrategy(Boolean
enableHybridTableRetentionStrategy) {
- _enableHybridTableRetentionStrategy = enableHybridTableRetentionStrategy;
- }
-
/// @deprecated Use `segmentIngestionFrequency` from
///
[org.apache.pinot.spi.config.table.ingestion.IngestionConfig#getBatchIngestionConfig()]
@Deprecated(since = "0.8.0", forRemoval = true)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
