From 5da68d7ff0f4d9000a03d38ae72eeabf12c5d97c Mon Sep 17 00:00:00 2001 From: aishikbh Date: Sun, 9 Jun 2024 09:53:23 +0530 Subject: [PATCH] Improve Retention Manager Segment Lineage Clean Up (#13232) --- .../helix/core/PinotHelixResourceManager.java | 17 ++++++++++------- .../helix/core/retention/RetentionManager.java | 2 +- 2 files changed, 11 insertions(+), 8 deletions(-) diff --git a/pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/PinotHelixResourceManager.java b/pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/PinotHelixResourceManager.java index 7b2acaab8971..c1d25eba1604 100644 --- a/pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/PinotHelixResourceManager.java +++ b/pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/PinotHelixResourceManager.java @@ -999,13 +999,16 @@ public synchronized PinotResourceManagerResponse deleteSegments(String tableName LOGGER.info("Trying to delete segments: {} from table: {} ", segmentNames, tableNameWithType); Preconditions.checkArgument(TableNameBuilder.isTableResource(tableNameWithType), "Table name: %s is not a valid table name with type suffix", tableNameWithType); - HelixHelper.removeSegmentsFromIdealState(_helixZkManager, tableNameWithType, segmentNames); - if (retentionPeriod != null) { - _segmentDeletionManager.deleteSegments(tableNameWithType, segmentNames, - TimeUtils.convertPeriodToMillis(retentionPeriod)); - } else { - TableConfig tableConfig = ZKMetadataProvider.getTableConfig(_propertyStore, tableNameWithType); - _segmentDeletionManager.deleteSegments(tableNameWithType, segmentNames, tableConfig); + + synchronized (getTableUpdaterLock(tableNameWithType)) { + HelixHelper.removeSegmentsFromIdealState(_helixZkManager, tableNameWithType, segmentNames); + if (retentionPeriod != null) { + _segmentDeletionManager.deleteSegments(tableNameWithType, segmentNames, + TimeUtils.convertPeriodToMillis(retentionPeriod)); + } else { + TableConfig tableConfig = ZKMetadataProvider.getTableConfig(_propertyStore, tableNameWithType); + _segmentDeletionManager.deleteSegments(tableNameWithType, segmentNames, tableConfig); + } } return PinotResourceManagerResponse.success("Segment " + segmentNames + " deleted"); } catch (final Exception e) { 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 dac61a3096e9..53075fc1333f 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 @@ -55,7 +55,7 @@ */ public class RetentionManager extends ControllerPeriodicTask { public static final long OLD_LLC_SEGMENTS_RETENTION_IN_MILLIS = TimeUnit.DAYS.toMillis(5L); - private static final RetryPolicy DEFAULT_RETRY_POLICY = RetryPolicies.exponentialBackoffRetryPolicy(5, 1000L, 2.0f); + private static final RetryPolicy DEFAULT_RETRY_POLICY = RetryPolicies.randomDelayRetryPolicy(20, 100L, 200L); private static final Logger LOGGER = LoggerFactory.getLogger(RetentionManager.class);