XuQianJin-Stars commented on code in PR #3518:
URL: https://github.com/apache/fluss/pull/3518#discussion_r3465663510
##########
fluss-lake/fluss-lake-hudi/src/test/java/org/apache/fluss/lake/hudi/testutils/FlinkHudiTieringTestBase.java:
##########
@@ -456,6 +460,30 @@ protected void checkFlussOffsetsInSnapshot(
}
}
+ protected void checkHudiCompactionCommitted(TablePath tablePath) {
+ retry(
+ Duration.ofMinutes(1),
+ () -> {
+ try (HudiTableInfo hudiTableInfo =
+ HudiTableInfo.create(
+ tablePath,
Configuration.fromMap(getHudiCatalogConf()))) {
+ HoodieTimeline timeline =
+ hudiTableInfo
+ .getMetaClient()
+ .getActiveTimeline()
+ .getCommitsAndCompactionTimeline()
+ .filterCompletedInstants();
+ assertThat(
+ timeline.getInstantsAsStream()
+ .anyMatch(
+ instant ->
+
isCompactionInstant(
+
timeline, instant)))
+ .isTrue();
+ }
Review Comment:
A 1-minute retry timeout on `checkHudiCompactionCommitted` is generous;
please reduce to 30s or expose as a constant. Also, `isCompactionInstant` reads
commit metadata and may throw `IOException`; if it does, the current
implementation propagates a `RuntimeException` and fails the retry. For a
polling helper it would be more lenient to `return false` on read errors and
let the next poll succeed.
##########
fluss-lake/fluss-lake-hudi/src/test/java/org/apache/fluss/lake/hudi/testutils/FlinkHudiTieringTestBase.java:
##########
@@ -521,6 +549,31 @@ private List<String> collectHudiRows(
}
}
+ private static List<FileSlice> getLatestFileSlicesAtCompletedInstant(
+ HudiTableInfo hudiTableInfo, String partition) {
+ HoodieTimeline completedTimeline =
hudiTableInfo.getCompletedTimeline();
+ HoodieInstant latestInstant =
+ completedTimeline
+ .lastInstant()
+ .orElseThrow(
+ () ->
+ new IllegalStateException(
+ "No completed Hudi instant
found for table "
+ +
hudiTableInfo.getTablePath()
+ + "."));
+ String latestInstantTime = latestInstant.requestedTime();
+ if (hudiTableInfo.getTableType() == HoodieTableType.MERGE_ON_READ) {
+ return hudiTableInfo
+ .getFileSystemView()
+ .getLatestMergedFileSlicesBeforeOrOn(partition,
latestInstantTime)
+ .collect(Collectors.toList());
+ }
+ return hudiTableInfo
+ .getFileSystemView()
+ .getLatestFileSlicesBeforeOrOn(partition, latestInstantTime,
true)
+ .collect(Collectors.toList());
Review Comment:
The MOR vs COW branches use different APIs
(`getLatestMergedFileSlicesBeforeOrOn` vs `getLatestFileSlicesBeforeOrOn(... ,
true)`) - please add a one-line comment explaining why
`includeFileSliceBefore=true` is needed for COW so future readers do not flip
it back.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
