You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by "shiyiky (via GitHub)" <gi...@apache.org> on 2024/07/02 16:58:44 UTC
Re: [PR] [FLINK-35674][cdc-connector][mysql]Fix blocking caused by searching for timestamp in binlog file [flink-cdc]
shiyiky commented on code in PR #3432:
URL: https://github.com/apache/flink-cdc/pull/3432#discussion_r1654671986
##########
flink-cdc-connect/flink-cdc-source-connectors/flink-connector-mysql-cdc/src/main/java/org/apache/flink/cdc/connectors/mysql/debezium/DebeziumUtils.java:
##########
Review Comment:
> Is it possible to create a mocked `BinlogClient` that fails to `connect()` for the first time and succeeds after retrying?
Error msg show that failed to register eventListene ,but BinlogClient connect is ok
##########
flink-cdc-connect/flink-cdc-source-connectors/flink-connector-mysql-cdc/src/main/java/org/apache/flink/cdc/connectors/mysql/debezium/DebeziumUtils.java:
##########
@@ -324,11 +324,22 @@ private static long getBinlogTimestamp(BinaryLogClient client, String binlogFile
client.setBinlogFilename(binlogFile);
client.setBinlogPosition(0);
- LOG.info("begin parse binlog: {}", binlogFile);
+ LOG.info("Begin parse binlog: {}", binlogFile);
client.connect();
} finally {
client.unregisterEventListener(eventListener);
}
+ if (binlogTimestamps.isEmpty()) {
+ try {
+ if (client.isConnected()) {
+ client.disconnect();
+ }
+ } catch (IOException e) {
+ throw new RuntimeException(e);
+ }
+ LOG.warn("Failed to register eventListener and try to register it again");
+ return getBinlogTimestamp(client, binlogFile);
Review Comment:
> What about using loop instead of recursion?
all the same .
##########
flink-cdc-connect/flink-cdc-source-connectors/flink-connector-mysql-cdc/src/main/java/org/apache/flink/cdc/connectors/mysql/debezium/DebeziumUtils.java:
##########
@@ -324,11 +324,22 @@ private static long getBinlogTimestamp(BinaryLogClient client, String binlogFile
client.setBinlogFilename(binlogFile);
client.setBinlogPosition(0);
- LOG.info("begin parse binlog: {}", binlogFile);
+ LOG.info("Begin parse binlog: {}", binlogFile);
client.connect();
} finally {
client.unregisterEventListener(eventListener);
}
+ if (binlogTimestamps.isEmpty()) {
+ try {
+ if (client.isConnected()) {
+ client.disconnect();
+ }
+ } catch (IOException e) {
+ throw new RuntimeException(e);
+ }
+ LOG.warn("Failed to register eventListener and try to register it again");
+ return getBinlogTimestamp(client, binlogFile);
Review Comment:
> Just worrying about overflow of calling stack. But since connection failure very unlikely to happen, I'll not insist on this.
Yes, you're right. I will adopt your opinion
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org