Skip to content

Commit 1d1d73d

Browse files
committed
address comments
1 parent 3754dcf commit 1d1d73d

1 file changed

Lines changed: 33 additions & 9 deletions

File tree

hudi-client/hudi-client-common/src/main/java/org/apache/hudi/table/action/ttl/strategy/KeepByEventTimeStrategy.java

Lines changed: 33 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020

2121
import org.apache.hudi.common.table.timeline.HoodieInstantTimeGenerator;
2222
import org.apache.hudi.common.util.PartitionPathEncodeUtils;
23+
import org.apache.hudi.common.util.collection.Pair;
2324
import org.apache.hudi.config.HoodieTTLConfig;
2425
import org.apache.hudi.keygen.constant.KeyGeneratorOptions;
2526
import org.apache.hudi.table.HoodieTable;
@@ -34,6 +35,7 @@
3435
import java.time.format.DateTimeFormatter;
3536
import java.time.format.DateTimeParseException;
3637
import java.time.temporal.TemporalAccessor;
38+
import java.util.Collections;
3739
import java.util.List;
3840
import java.util.stream.Collectors;
3941

@@ -158,16 +160,38 @@ protected List<String> getExpiredPartitionsForTimeStrategy(List<String> partitio
158160
long cutoffMillis = resolveCutoffMillis(instantTime, ttlInMilis);
159161
DateTimeFormatter formatter = DateTimeFormatter.ofPattern(eventTimeFormat).withZone(ZoneOffset.UTC);
160162
int segCount = segmentCount(eventTimeFormat);
161-
return partitionPathsForTTL.stream().parallel()
163+
164+
// Step 1: filter by event time. Pure computation over immutable inputs (no shared state, no
165+
// I/O), so a raw parallel stream on the common ForkJoinPool is safe. In a healthy table the
166+
// vast majority of partitions survive this filter, so most of the work stays here and never
167+
// reaches the more expensive step 2.
168+
List<String> eventTimeExpired = partitionPathsForTTL.stream().parallel()
162169
.filter(path -> isPartitionExpiredByEventTime(
163-
path, formatter, timeSegStartIndex, segCount, cutoffMillis, shouldDeleteHiveDefaultPartition, hiveStylePartitioning))
164-
// A partition emptied by a previous TTL replace commit keeps showing up in
165-
// getAllPartitionPaths() until the cleaner physically removes it, and its event time is
166-
// derived from the (unchanged) path, so it would be re-selected on every batch -- issuing
167-
// an empty replace commit each time and never converging. Keep only partitions that still
168-
// have a live file slice so the strategy is idempotent, mirroring KeepByTimeStrategy which
169-
// keys off the surviving slices' last commit time and thus naturally skips deleted ones.
170-
.filter(this::hasLiveFileSlice)
170+
path, formatter, timeSegStartIndex, segCount, cutoffMillis,
171+
shouldDeleteHiveDefaultPartition, hiveStylePartitioning))
172+
.collect(Collectors.toList());
173+
if (eventTimeExpired.isEmpty()) {
174+
return Collections.emptyList();
175+
}
176+
177+
// Step 2: drop partitions that an earlier TTL replace already emptied. Their event time is
178+
// derived from the (unchanged) path, so without this filter they would be re-selected on
179+
// every batch -- issuing an empty replace commit each time and never converging. Keep only
180+
// partitions that still have a live file slice, mirroring KeepByTimeStrategy which keys off
181+
// the surviving slices' last commit time and thus naturally skips deleted ones.
182+
//
183+
// hasLiveFileSlice touches hoodieTable.getHoodieView(), which for non-default view types
184+
// (SPILLABLE_DISK, EMBEDDED_KV_STORE) mutates a backing store during first-time partition
185+
// load under a shared read lock -- unsafe under unbounded concurrent access on the common
186+
// ForkJoinPool. Route the lookup through the engine context with bounded parallelism, the
187+
// same pattern KeepByTimeStrategy#getLastCommitTimeForPartitions uses, so that blocking file
188+
// listings don't run on the common pool and respect getPartitionTTLStatsMaxParallelism.
189+
int statsParallelism = Math.min(eventTimeExpired.size(), writeConfig.getPartitionTTLStatsMaxParallelism());
190+
return hoodieTable.getContext().map(eventTimeExpired,
191+
partitionPath -> Pair.of(partitionPath, hasLiveFileSlice(partitionPath)),
192+
statsParallelism).stream()
193+
.filter(Pair::getRight)
194+
.map(Pair::getLeft)
171195
.collect(Collectors.toList());
172196
}
173197

0 commit comments

Comments
 (0)