XuQianJin-Stars commented on code in PR #3518:
URL: https://github.com/apache/fluss/pull/3518#discussion_r3465663453
##########
fluss-lake/fluss-lake-hudi/src/main/java/org/apache/fluss/lake/hudi/tiering/HudiLakeCommitter.java:
##########
@@ -155,22 +157,24 @@ public void abort(HudiCommittable committable) throws
IOException {
"Failed to abort Hudi instant " +
instant + ".", e));
}
}
+ for (String instant : committable.getCompactionWriteStats().keySet()) {
+ try {
+ abortCompactionInstant(instant);
Review Comment:
Ordering: data instants are aborted first and compaction instants second. If
a compaction was scheduled against base files produced by the data instant
being aborted, rolling back data first leaves the inflight compaction
referencing a missing base file, and the subsequent rollback may fail or
corrupt the timeline. Suggest aborting compaction instants first, then data
instants, and add a unit test that exercises both maps populated.
##########
fluss-lake/fluss-lake-hudi/src/main/java/org/apache/fluss/lake/hudi/tiering/HudiLakeCommitter.java:
##########
@@ -207,6 +211,53 @@ public void close() throws Exception {
IOUtils.closeQuietly(hudiTableInfo, "hudi table info");
}
+ @Nullable
+ private String commitCompactionAndSchedule(
+ Map<String, HudiWriteStats> compactionWriteStats, Map<String,
String> commitMetadata) {
+ if (!isAutoCompactionEnabled()) {
+ return null;
+ }
+
+ String latestCommittedInstant = null;
+ try {
+ latestCommittedInstant =
+ compactionService.commitCompaction(compactionWriteStats,
commitMetadata);
+ if (latestCommittedInstant != null) {
+ LOG.info("Committed Hudi compaction instant {}.",
latestCommittedInstant);
+ }
+ } catch (Exception e) {
Review Comment:
All exceptions from `commitCompaction` are reduced to a WARN. Combined with
the WARN at line 240 for `scheduleCompaction`, repeated compaction failures are
entirely invisible from the outside. Please either:
- propagate, or
- expose a numeric metric (`compactionCommitFailures`,
`compactionScheduleFailures`) on `LakeCommitter` so operators can alert on them.
A log-only failure surface for a long-running tiering service will be very
hard to operate.
--
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]
