1996fanrui commented on code in PR #24757:
URL: https://github.com/apache/flink/pull/24757#discussion_r1612639848
##########
flink-runtime/src/main/java/org/apache/flink/runtime/source/coordinator/SourceCoordinator.java:
##########
@@ -201,8 +201,11 @@ void announceCombinedWatermark() {
// to ready task to avoid period task fail (Java-ThreadPoolExecutor
will not schedule
// the period task if it throws an exception).
for (Integer subtaskId : subTaskIds) {
- context.sendEventToSourceOperatorIfTaskReady(
- subtaskId, new
WatermarkAlignmentEvent(maxAllowedWatermark));
+ // when subtask have been finished, do not send event.
+ if (!context.hasNoMoreSplits(subtaskId)) {
Review Comment:
> I believe these two locations should be equivalent.
I think you are right, both of `DataInputStatus.END_OF_INPUT` and
`NoMoreSplitsEvent` work well for now.
I prefer `DataInputStatus.END_OF_INPUT` because its semantics are clearer,
and it's closer to FINISHED. It means the task will be definitely switched to
FINISHED after `DataInputStatus.END_OF_INPUT`.
As we all know, code is often refactored. We shouldn't make assumptions
(especially assumptions that other developers don't know about). Other
developers may break our assumption if they don't know during the refactor in
the future. I'm afraid the `NoMoreSplitsEvent` is used for other cases, and
task doesn't switch to FINISHED after receiving `NoMoreSplitsEvent` in the
future. That's my concern.
--
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]
