[flink] Support end input in coordinator commit - #8671
[flink] Support end input in coordinator commit#8671fishfishfishfishaa wants to merge 11 commits into
Conversation
|
- Defer coordinator Context initialization - Pass actual streamingCheckpointEnabled from FlinkSink - Sync for END_INPUT event completion in batch mode - Remove unnecessary empty snapshot check in streaming IT
|
Regarding the ITCase failure in batch mode: due to the asynchronous architecture of the coordinator, the batch job finishes first, and close() interrupts the final commit. I've adjusted it to use synchronous blocking for batch EndInput events. Since this is not waiting for the last EndInput event — it only waits for in-memory operations — the blocking time is minimal. Moreover, EndInput itself is in the "finalization" phase. Compared to other approaches, I believe this design is simpler and more consistent with the original logic. (Other alternatives include: 1. Modifying close() to wait for all pending commits; 2. Explicitly waiting only for the last one; 3. Adding an ack mechanism.) Additionally, the coordinator Context initialization issue has also been fixed. |
"the batch coordinator path bypasses BatchWriteGeneratorTagOperator, and tag.automatic-creation=batch silently fails to create a tag" still not fix yet |
|
…l operator commit logic
|
For batch automatic tag, I have added validation to prevent silent failures. I will complete the support for this in a follow-up, and it will be tracked later in #8220. @ifndef-SleePy PTAL and let me know if you have any suggestions. Thanks! |
|
Hi @fishfishfishfishaa , sorry for late response. I hive two design concerns about this PR:
A few small suggestions:
|
|
After all subtasks have issued END_INPUT, |
# Conflicts: # paimon-flink/paimon-flink-common/src/main/java/org/apache/paimon/flink/sink/CoordinatorCommittingRowDataStoreWriteOperator.java # paimon-flink/paimon-flink-common/src/main/java/org/apache/paimon/flink/sink/FlinkSink.java # paimon-flink/paimon-flink-common/src/main/java/org/apache/paimon/flink/sink/coordinator/CommittingWriteOperatorCoordinator.java # paimon-flink/paimon-flink-common/src/test/java/org/apache/paimon/flink/sink/CoordinatorCommittingRowDataStoreWriteOperatorTest.java
Normal scenariosWriters reach EndInput at different checkpointsAn early EndInput writer keeps its UT:
EndInput arrives after an already completed checkpointA bounded writer may produce its final data and EndInput only after the coordinator has already This path does not require Flink to schedule another checkpoint after bounded tasks finish. In UT&IT:
Checkpoint completion after the final EndInput commitIf the last EndInput event is processed before a later checkpoint completion notification, it UT:
Repeated EndInput before the final commitEndInput may be replayed after recovery. The latest event is treated as the authoritative state for UT:
Repeated EndInput after the final commitWhen UT:
Failure scenariosCheckpoint completion overtakes a delayed EndInput eventA writer may have already reached EndInput and persisted its The old attempt's delayed event is not used as recovery authority. After failover, each writer The IT controls checkpoint triggering only to reproduce this ordering deterministically. The UT&IT:
WriteTask FailoverWriter task fails before EndInputThe coordinator remains in UT:
A writer task fails after EndInput
While the coordinator is still
UT:
All writers EndInput before another checkpoint completionThe EndInput entries are stored in writer operator state. After writer recovery, the restore events UT:
JM FailoverJM failover with partial EndInputThe recreated coordinator enters When only some writers contain EndInput:
UT:
JM failover with all writers at EndInputWhen all restore events contain EndInput, If recovery creates a missing snapshot, the configured recovery behavior triggers another job UT:
Batch Mode JM failover without a completed checkpointWithout a completed checkpoint, there is no writer pending state to restore. Batch scheduling UT&IT:
Writer task and JM failover happen together or in sequenceJM failover subsumes a writer region failover that is still running or has just completed. In
UT:
CK AbortCheckpoint abort after partial EndInputAn aborted ordinary checkpoint does not remove pending committables. A later completed checkpoint UT:
|
|
There is a race condition between |
Good catch. Even with direct final commit after all The current implementation addresses this in two ways:
I added two ITs:
The current implementation ensures that the final commit is not lost. The race may trigger failover, but it does not affect consistency. |
|
Before diving into implementation details, I'd like to discuss the design from a high-level view first. Race condition issue:The original
This PR instead always triggers the commit directly on end input. For the streaming + checkpointing-enabled case, this differs from There is also the global failover case: the in-memory Another case: if a writer calls Failover on finished task:For a writer task that has already sent its end input, is there any scenario where it would not be brought up again to re-send |
|
Furthermore, the complexity of the current implementation comes from trying to support batch and streaming at the same time, and the fact that the data interaction and failover handling in coordinator commit is more complex than in operator commit.
PIP-30 is meant to solve the region failover problem in the streaming case, so we could discuss whether it's necessary to support batch and streaming-without-checkpoint at all.
|
|
One more addition: the most important reason coordinator commit introduces end input is for testability. In most of PIP-30's scenarios, end input is never triggered, so increasing the complexity of the main logic just to support end input doesn't seem worthwhile. Based on our internal practice, without end input support it's fairly hard to implement the failover IT cases: while injecting random task failover and global failover, we need to verify that both the Flink and Paimon jobs recover normally and that data consistency holds against a fixed dataset. |
|
Thanks for pointing this out. The main motivation for supporting EndInput in coordinator commit was testability. However, the current implementation directly uses EndInput to avoid final commit loss, which indeed introduces additional complexity in both commit semantics and recovery. I also agree that, for the PIP-30 goal of supporting region failover, batch mode and streaming without checkpointing provide limited practical value. Based on this, I will try to simplify the implementation, narrow the supported scope, and remove the extra EndInput direct-commit path so that the commit and recovery semantics remain clearer. |
Purpose
This PR implements the end-of-input handling that was intentionally left out of the core coordinator-commit PR for [PIP-30 #8220 ].
It extends
sink.coordinator-commit.enabledto correctly finish and commit bounded inputs. In particular, coordinator commit now supports batch jobs without checkpointing, and correctly handles bounded streaming jobs after all writers reachendInput.This PR is built on top of the core coordinator-commit implementation. It does not change the default committer-operator path.
What changes
Commit after all writers finish
Each coordinated writer emits a final committable entry with
Long.MAX_VALUEas a dedicated end-of-input checkpoint ID when Flink invokesendInput.The JobManager-side
CommittingWriteOperatorCoordinatortracks this entry separately from ordinary checkpoint committables:filterAndCommit(..., false, true).For streaming jobs with checkpointing enabled, the final commit remains aligned with checkpoint completion. This preserves the normal checkpoint-driven commit model while allowing some subtasks to finish before others.
For batch jobs, where checkpointing is normally disabled, the coordinator commits immediately after receiving end-input entries from all writer subtasks.
Support batch coordinator commit
Coordinator commit previously required streaming mode with checkpointing enabled. This is now relaxed as follows:
The existing coordinator-commit restrictions remain unchanged: it only supports write-only unaware-bucket append tables, and still rejects configurations such as primary-key tables, precommit compaction, auto-tag-for-savepoint, and concurrent checkpoints.
The option documentation is updated accordingly.
Preserve end-input state across recovery
End input is not an ordinary checkpoint: it is newer than every checkpoint and may need to survive restore even if Flink does not invoke
endInputagain after recovery.To make this safe:
endInputis invoked again after restore, the writer merges newly produced final committables with the persisted entry and sends one authoritative final entry.WriterCommittablestreats end input separately from the maximum ordinary checkpoint, so it does not incorrectly constrain ordinary checkpoint alignment.This also handles region failover while the coordinator is still waiting for the remaining writers to finish.
End-input watermark handling
The writer now uses
end-input.watermark, when configured, for the final end-input committable.For ordinary checkpoints after a subtask has finished, that subtask contributes
Long.MAX_VALUEto the watermark minimum. This makes a finished writer neutral and prevents it from incorrectly holding back the watermark of active writers.The final commit uses the configured end-input watermark as expected.
How it works
Tests
This PR adds coverage for:
end-input.watermark.WriterCommittablessemantics:No production behavior changes when
sink.coordinator-commit.enabledis disabled.