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 2b0da67bc43 Extract hybrid table retention check into overridable
shouldEnableHybridTableRetentionStrategy method (#20088)
2b0da67bc43 is described below
commit 2b0da67bc437b139e50363ea438bdc38070647f2
Author: Manak Bisht <[email protected]>
AuthorDate: Mon Nov 2 12:44:43 2026 +0530
Extract hybrid table retention check into overridable
shouldEnableHybridTableRetentionStrategy method (#20088)
* Extract hybrid table retention check into overridable
shouldEnableHybridTableRetentionStrategy method
* Document the `shouldEnableHybridTableRetentionStrategy` overridable method
* Test that the shouldEnableHybridTableRetentionStrategy override controls
hybrid table retention routing
---
.../helix/core/retention/RetentionManager.java | 15 +++++++-
.../helix/core/retention/RetentionManagerTest.java | 45 ++++++++++++++++++++++
2 files changed, 58 insertions(+), 2 deletions(-)
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 e2a65a92098..72d47f8b82e 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
@@ -360,7 +360,7 @@ public class RetentionManager extends
ControllerPeriodicTask<Void> {
String rawTableName =
TableNameBuilder.extractRawTableName(tableNameWithType);
TableConfig offlineTableConfig =
_pinotHelixResourceManager.getOfflineTableConfig(rawTableName);
// hybrid table check should be performed before the realtime table
check.
- if (_isHybridTableRetentionStrategyEnabled && offlineTableConfig !=
null) {
+ if (offlineTableConfig != null &&
shouldEnableHybridTableRetentionStrategy(tableConfig, offlineTableConfig)) {
// TODO: handle the orphan segment deletion for hybrid table
manageRetentionForHybridTable(tableConfig, offlineTableConfig);
} else {
@@ -370,6 +370,16 @@ public class RetentionManager extends
ControllerPeriodicTask<Void> {
}
}
+ /// Returns whether the hybrid table retention strategy should be applied to
the given REALTIME table. Subclasses can
+ /// override this to decide per table.
+ ///
+ /// The default returns the value of
[ControllerConf#ENABLE_HYBRID_TABLE_RETENTION_STRATEGY], which can be updated at
+ /// runtime through cluster config.
+ protected boolean shouldEnableHybridTableRetentionStrategy(TableConfig
realtimeTableConfig,
+ TableConfig offlineTableConfig) {
+ return _isHybridTableRetentionStrategyEnabled;
+ }
+
private void manageRetentionForOfflineTable(String offlineTableName,
RetentionStrategy retentionStrategy,
int untrackedSegmentsDeletionBatchSize, RetentionStrategy
untrackedSegmentsRetentionStrategy) {
List<SegmentZKMetadata> segmentZKMetadataList =
_pinotHelixResourceManager.getSegmentsZKMetadata(offlineTableName);
@@ -392,7 +402,8 @@ public class RetentionManager extends
ControllerPeriodicTask<Void> {
}
}
- private void manageRetentionForRealtimeTable(String realtimeTableName,
RetentionStrategy retentionStrategy,
+ @VisibleForTesting
+ 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 a442d598587..4396a5b5b24 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
@@ -515,6 +515,51 @@ public class RetentionManagerTest {
verify(mockPinotHelixResourceManager,
times(1)).deleteSegments(eq(realtimeTableName), anyList());
}
+ @Test
+ public void testHybridTableRetentionOverrideTakesPrecedenceOverConfig() {
+ TableConfig realtimeTableConfig = createRealtimeTableConfig1(1);
+ TableConfig offlineTableConfig = createOfflineTableConfig();
+ PinotHelixResourceManager resourceManager =
mock(PinotHelixResourceManager.class);
+
when(resourceManager.getOfflineTableConfig(TEST_TABLE_NAME)).thenReturn(offlineTableConfig);
+ ControllerConf conf = new ControllerConf();
+ conf.setProperty(ControllerConf.ENABLE_HYBRID_TABLE_RETENTION_STRATEGY,
false);
+ RetentionManager retentionManager =
+ createRetentionManagerWithHybridRetentionOverride(realtimeTableConfig,
resourceManager, conf);
+
+ retentionManager.processTable(REALTIME_TABLE_NAME);
+
+
verify(retentionManager).shouldEnableHybridTableRetentionStrategy(realtimeTableConfig,
offlineTableConfig);
+
verify(retentionManager).manageRetentionForHybridTable(realtimeTableConfig,
offlineTableConfig);
+ verify(retentionManager,
never()).manageRetentionForRealtimeTable(anyString(), any(), anyInt(), any());
+ }
+
+ @Test
+ public void testHybridTableRetentionOverrideNotCalledWithoutOfflineTable() {
+ TableConfig realtimeTableConfig = createRealtimeTableConfig1(1);
+ PinotHelixResourceManager resourceManager =
mock(PinotHelixResourceManager.class);
+
when(resourceManager.getOfflineTableConfig(TEST_TABLE_NAME)).thenReturn(null);
+ RetentionManager retentionManager =
+ createRetentionManagerWithHybridRetentionOverride(realtimeTableConfig,
resourceManager, new ControllerConf());
+
+ retentionManager.processTable(REALTIME_TABLE_NAME);
+
+ verify(retentionManager,
never()).shouldEnableHybridTableRetentionStrategy(any(), any());
+ verify(retentionManager, never()).manageRetentionForHybridTable(any(),
any());
+
verify(retentionManager).manageRetentionForRealtimeTable(eq(REALTIME_TABLE_NAME),
any(), anyInt(), any());
+ }
+
+ /// Returns a spy of a real retention manager whose
`shouldEnableHybridTableRetentionStrategy` always returns `true`,
+ /// simulating a subclass that overrides it. All other methods run their
real implementation.
+ private RetentionManager
createRetentionManagerWithHybridRetentionOverride(TableConfig
realtimeTableConfig,
+ PinotHelixResourceManager resourceManager, ControllerConf conf) {
+ // The realtime retention path reads the ideal state through the Helix
admin
+ when(resourceManager.getHelixAdmin()).thenReturn(mock(HelixAdmin.class));
+ RetentionManager retentionManager =
spy(createSizeRetentionManager(realtimeTableConfig, List.of(), resourceManager,
+ mock(ControllerMetrics.class), conf, mock(BrokerServiceHelper.class)));
+
doReturn(true).when(retentionManager).shouldEnableHybridTableRetentionStrategy(any(),
any());
+ return retentionManager;
+ }
+
private TableConfig createOfflineTableConfig() {
return new
TableConfigBuilder(TableType.OFFLINE).setTableName(TEST_TABLE_NAME).setRetentionTimeUnit("DAYS")
.setRetentionTimeValue("365").setNumReplicas(2).build();
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
