chengcongchina commented on code in PR #4334:
URL: https://github.com/apache/flink-cdc/pull/4334#discussion_r2985553842
##########
flink-cdc-connect/flink-cdc-source-connectors/flink-connector-mysql-cdc/src/main/java/io/debezium/connector/mysql/MySqlStreamingChangeEventSource.java:
##########
@@ -1264,6 +1272,13 @@ public void execute(
} catch (Exception e) {
LOGGER.info("Exception while stopping binary log client", e);
}
+
+ client.unregisterEventListener(eventListener);
+ client.unregisterEventListener(metricsEventListener);
+ client.unregisterLifecycleListener(lifecycleListener);
+ if (logEventListener != null) {
+ client.unregisterEventListener(logEventListener);
Review Comment:
Thanks for the review. I agree that if unregister*() throws in finally , it
can mask the original exception and make the root cause harder to diagnose.
After reconsideration, I moved the listener unregistration to the end of the
normal execution path instead of the finally block. The reason is that the
problematic case we want to avoid is cross-split reuse when the execution
finishes normally; if an exception happens and we exit early, the task will
fail and the BinaryLogClient will be recreated on recovery, so the listener
accumulation issue should not be hit in that path.
--
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]
