osheroff / mysql-binlog-connector-java

MySQL Binary Log connector
680 stars 167 forks source link

How about skip Fake rotate Event When connection resumed #126

Open sean-k1 opened 1 year ago

sean-k1 commented 1 year ago

@osheroff https://github.com/debezium/debezium/pull/4959

debezium users When the connection to the Mysql database is lost, a fake Rotate Event occurs in communication with the master. so they got Fake Rotate Event. These issues are causing inconvenience to users in debezium.

What do you think about ignoring the fake rotate event in this project?

https://mariadb.com/kb/en/fake-rotate_event/ https://github.com/go-mysql-org/go-mysql/blob/3a75f6a26d8819c2a88629e54c7de0b7698ae05c/replication/backup.go#L59

Naros commented 1 year ago

To be clear @sean-k1, this isn't an issue for MySQL but rather precisely for MariaDB, correct?

sean-k1 commented 1 year ago

@Naros No. this issue is related with Mysql and Maraidb,

If the binlogfile is filled to size by max_binlog_size, the a rotate Event is fired at the end of the current binlogfile, then a fake rotate Event is occured at the beginning of the next binlogfile.

This means that the first event when communicating with the master will be a rotate event. The fake rotate event is occured because we are requesting a next(new) binlogfile from the master, and the same is when the connection to the database is lost.

My current understanding is this.

check this one. My env is Mysql 8.0

스크린샷 2023-10-25 오후 9 26 57 스크린샷 2023-10-25 오후 9 26 33
sean-k1 commented 1 year ago

@Naros Did you check this one?

Naros commented 12 months ago

Hi @sean-k1, I'm on the fence with this one.

In particular, is there a scenario where a client getting this fake event may be using it for some processing? If we exclude it generally from the event stream, that could break that compatibility.

Looking at the source for both MariaDB and MySQL, the LOG_EVENT_ARTIFICIAL_F flag is only ever used for this specific fake ROTATE_EVENT, so I'm curious if perhaps it may be satisfactory to include a toggle to exclude artificial events from being dispatched to event listeners and only consumed by the BinlogClient itself, i.e.:

                if (isConnected()) {
                    eventLastSeen = System.currentTimeMillis();
                    updateGtidSet(event);
                    if (dispatchArtificialEvents || !isArtificialEvent(event)) {
                        notifyEventListeners(event);
                    }                 
                    updateClientBinlogFilenameAndPosition(event);
                }

What's your take on this?

sean-k1 commented 12 months ago

Hello @Naros I think your solution is pretty cool. but I'm curious, why is dispatchArtificialEvents as a variable?

Naros commented 12 months ago

It's there mostly to preserve existing behavior. I think it's something we can re-evaluate later on whether we should hard enforce no delivery without being able to toggle it on/off later based on feedback.

For Debezium, what we would do is set the toggle to false (true remains compatible with current behavior) and we could safely remove your temporary fix in Debezium and we would no longer see this event in the Debezium event callbacks.

sean-k1 commented 12 months ago

@Naros Oh I understood. Thanks you for the explanation.