XuQianJin-Stars commented on code in PR #3518:
URL: https://github.com/apache/fluss/pull/3518#discussion_r3465663496
##########
fluss-lake/fluss-lake-hudi/src/main/java/org/apache/fluss/lake/hudi/utils/HudiTableInfo.java:
##########
@@ -111,7 +111,9 @@ public static HudiTableInfo create(TablePath tablePath,
Configuration hudiConfig
HoodieTableFileSystemView fileSystemView = null;
HoodieTableMetaClient metaClient = createMetaClient(basePath,
hudiConfig);
HoodieTimeline completedTimeline =
- metaClient.getCommitsTimeline().filterCompletedInstants();
+ metaClient
+ .getCommitsAndCompactionTimeline()
+ .filterCompletedAndCompactionInstants();
Review Comment:
The semantics of `completedTimeline` have been silently changed from
'completed commits' to 'completed commits + any compaction state'.
`HudiSplitPlanner.java:62` uses
`getCompletedTimeline().containsInstant(snapshotTime)` to decide whether a
snapshot is readable. With the new definition, an inflight or requested
compaction instant will be reported as readable, which can lead a reader to
plan splits over partial state.
Please keep `completedTimeline` strictly completed-only and introduce a
separate accessor (e.g. `getFileSystemViewTimeline()`) for the file-system view
that needs the broader timeline. The two consumers have different correctness
requirements.
##########
fluss-lake/fluss-lake-hudi/src/test/java/org/apache/fluss/lake/hudi/testutils/FlinkHudiTieringTestBase.java:
##########
@@ -187,6 +189,7 @@ protected long createPkTable(
if (enableAutoCompaction) {
tableBuilder.property(ConfigOptions.TABLE_DATALAKE_AUTO_COMPACTION.key(),
"true");
Review Comment:
`"hudi."` is repeated in several places in this PR. Please extract a
constant (the same prefix is implicitly defined elsewhere when Fluss forwards
table options into Hudi config; reusing the existing constant would prevent
drift).
--
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]
