AHeise commented on code in PR #25547:
URL: https://github.com/apache/flink/pull/25547#discussion_r1819084931
##########
flink-runtime/src/main/java/org/apache/flink/streaming/api/connector/sink2/CommittableSummary.java:
##########
@@ -39,11 +39,21 @@ public class CommittableSummary<CommT> implements
CommittableMessage<CommT> {
private final long checkpointId;
/** The number of committables coming from the given subtask in the
particular checkpoint. */
private final int numberOfCommittables;
+
+ @Deprecated
/** The number of committables that have not been successfully committed.
*/
private final int numberOfPendingCommittables;
+
+ @Deprecated
/** The number of committables that are not retried and have been failed.
*/
private final int numberOfFailedCommittables;
Review Comment:
Good point. I probably should motivate that change in the commit message
first. The short answer is: it doesn't work. Let's go to the long one.
So far, we increase this number on known issue and don't emit the respective
committable. That means that the global committer would need to wait for all
committables except the failed once and then commit. However, it always used to
wait for all known committables running in an infinite loop.
The change of this PR only creates the summary of successful commits.
Unknown errors cause a restart loop and known errors cause the committables to
be dropped from the statistics. So the global committer waits for all
committables of the summary and works now.
The alternative would be to still update the stastics as you propose and
ignore the failed in the global committer. However, I wonder what the value of
that is. If you'd like to keep it just to have less disruptions, I can revert
the change and fix it.
However, the custom topologies that I have seen so far, also run into the
same issue as the global committer (and now I think that the compactor has the
same issues). I think that emitting the stats on the ignored committables
ultimately just increases complexity without giving downstream operators any
good handle whatsoever.
WDYT?
--
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]
