driessamyn-sparta commented on PR #29676: URL: https://github.com/apache/flink/pull/29676#issuecomment-6529149780
Thanks @pnowojski and @MartijnVisser @pnowojski , on the motivation. We run a Flink 2.2.0 job with several `KafkaSink`s (at-least-once, flink-connector-kafka 5.0.0-2.2) on Kubernetes. A Kafka delivery timeout failed a task. `KafkaWriter#close()` then rethrew the pending async send failure as an IOException, and that reached `notifyFatalError`. I agree with the principle that a `close()` which fails to release its resources should not be ignored, and the connector's rethrow is old (unchanged since 3.0.2). In this case, as Martijn found, `KafkaWriter#close` closes the producer before it rethrows, so the exception reports an earlier send failure rather than a failed release. A task failure that was already going to restart the job then also takes down the TaskManager and any other job on it. I have not reproduced how the async exception is still pending at `close()` time. My reading of the connector source is that a later callback in the same expired batch sets it again after the first one was thrown. I see two ways forward and would like your view on which you prefer: 1. Keep this PR's approach: suppress and log a failing `cleanUp` after a task failure, as in 2.1 and earlier, with JVM-fatal errors still fatal. 2. Keep the FLINK-38180 behaviour and fix the cause in flink-connector-kafka, for example by not rethrowing from `close()` an async error that has already failed the task. I have not checked whether that is safe for the non-failure close paths, so I would need guidance there. I will not change PR #29676 until we have agreed which direction to take. Once we do, I will address Martijn's review comments: the Javadoc wording, documenting `TaskInvokable#cleanUp`, the OutOfMemoryError handling, and a test for the self-suppression guard. If option 2 is preferred I will withdraw the PR and open a connector issue instead. -- 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]
