Fix bug in handling sub events of replication.TransactionPayloadEvent

froot commented 2 months ago

When using the canal package with a MySQL instance with binlog-transaction-compression=ON, the following error occurs when you execute an INSERT statement on a table:

[2024/05/07 17:30:01] [info] sync.go:237 table structure changed, clear table cache: test_database.employees
panic: interface conversion: replication.Event is *replication.QueryEvent, not *replication.RowsEvent

goroutine 1 [running]:
github.com/go-mysql-org/go-mysql/canal.(*Canal).handleRowsEvent(0x140002ee0e0?, 0x1400049a180?)
    /Users/lulusheng/go/pkg/mod/github.com/go-mysql-org/go-mysql@v1.7.1-0.20240507101235-8d4de0fa6687/canal/sync.go:253 +0x2a4
github.com/go-mysql-org/go-mysql/canal.(*Canal).runSyncBinlog(0x140002ee0e0)
    /Users/lulusheng/go/pkg/mod/github.com/go-mysql-org/go-mysql@v1.7.1-0.20240507101235-8d4de0fa6687/canal/sync.go:107 +0x9fc
github.com/go-mysql-org/go-mysql/canal.(*Canal).run(0x140002ee0e0)
    /Users/lulusheng/go/pkg/mod/github.com/go-mysql-org/go-mysql@v1.7.1-0.20240507101235-8d4de0fa6687/canal/canal.go:239 +0xdc
github.com/go-mysql-org/go-mysql/canal.(*Canal).Run(...)
    /Users/lulusheng/go/pkg/mod/github.com/go-mysql-org/go-mysql@v1.7.1-0.20240507101235-8d4de0fa6687/canal/canal.go:194
main.main()
    /Users/lulusheng/Desktop/mysql-binlog-hacking/main.go:42 +0x194

This is because the runSyncBinlog function assumes that the sub events of a replication.TransactionPayloadEvent event are all row events. This isn't true (you can see an example of this in this PR comment https://github.com/go-mysql-org/go-mysql/pull/773#issuecomment-1440776208). This PR addresses this issue by making the event handling logic its own method and calling it recursively from the TransactionPayloadEvent case.

This PR is easier to review if you hide the whitespace.

lance6716 commented 2 months ago

lgtm.

I noticed in https://github.com/go-mysql-org/go-mysql/pull/773#issuecomment-1440776208 the header of subevents are not the same as a normal binlog events, and EventHandler interface will pass that header to the caller. Does your application want more change? like filling the log position for subevents.

froot commented 2 months ago

Yeah I noticed the Log position: 0. For my application, we don't use that field so it doesn't affect us.

It seems like mysqlbinlog will output the end position of the Transaction_payload_event for all the sub events:

# at 886
#240508 14:41:28 server id 2  end_log_pos 1094 CRC32 0x12e4f78b         Transaction_Payload             payload_size=175       compression_type=ZSTD   uncompressed_size=235
# Start of compressed events.
# at 1094
#240508 14:41:28 server id 2  end_log_pos 1094 CRC32 0x66d1b0d9         Query   thread_id=8     exec_time=0     error_code=0
SET TIMESTAMP=1715193688/*!*/;
BEGIN
/*!*/;
# at 1094
#240508 14:41:28 server id 2  end_log_pos 1094 CRC32 0x14acda3c         Table_map: `test_database`.`employees` mapped to number 92
# has_generated_invisible_primary_key=0
# at 1094
#240508 14:41:28 server id 2  end_log_pos 1094 CRC32 0xae348986         Write_rows: table id 92 flags: STMT_END_F
[...]

We could probably mimic the same approach when creating the subevents of a transaction payload event. Anyways, I don't think it's necessary for this PR in particular. Are you good to approve @lance6716 ?

Also, I noticed that there hasn't been a new release in a while. How difficult would it be to cut a new release soon?

lance6716 commented 2 months ago

@atercattus Hi, do you have time to release a new version? In fact I'm not familiar with the procedure.

atercattus commented 2 months ago

Of course. Should have done this a long time ago :)

go-mysql-org / go-mysql

Fix bug in handling sub events of replication.TransactionPayloadEvent #875