Skip to content

Add GTIDEvent interface to get the SequenceNumber and LastCommitted #818

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Sep 13, 2023

Conversation

cameron-p-m
Copy link
Contributor

@cameron-p-m cameron-p-m commented Sep 1, 2023

Problem

We've implemented MySQL Parallel Replication on our application that uses a fork of this library, and would like to merge the changes upstream.
Here are some details explaining it: https://github.com/isotopp/mysql-dev-examples/blob/master/mysql-binlog-parallel/parallel.md

We need to get the SequenceNumber and LastCommitted in order to do so. This is what I thought was the best way. Will have to be part of the next major release because it changes the interface of the handler.

@lance6716
Copy link
Collaborator

We need to get the SequenceNumber and LastCommitted in order to do so

How about we expose SequenceNumber and LastCommitted as the common methods of GTIDEvent interface? Seems MariaDB does not have LastCommitted so we can use SequenceNumber-1 (I will learn it later)

And also an ID() string method which returns server UUID / domain ID to distinguish binlog events from different primary node that can be applied concurrently.

@cameron-p-m
Copy link
Contributor Author

And also an ID() string method which returns server UUID / domain ID to distinguish binlog events from different primary node that can be applied concurrently.

What topology would this be useful? I don't think this would be common enough to be useful for MySQL in most cases.

How about we expose SequenceNumber and LastCommitted as the common methods of GTIDEvent interface? Seems MariaDB does not have LastCommitted so we can use SequenceNumber-1 (I will learn it later)

Sure, I can remove Decode and add these ones.

@cameron-p-m
Copy link
Contributor Author

How about we expose SequenceNumber and LastCommitted as the common methods of GTIDEvent interface? Seems MariaDB does not have LastCommitted so we can use SequenceNumber-1 (I will learn it later)

I did this, but it doesn't really work because of MariaDBs unsigned int64 vs MySQL signed int64. The changes are pushed so you can take a look.

@lance6716
Copy link
Collaborator

seems sequence_number can be a negative value 😢

https://github.com/mysql/mysql-server/blob/ea1efa9822d81044b726aab20c857d5e1b7e046a/sql/rpl_trx_tracking.cc#L151-L163

I'll check MariaDB later. Maybe unify them is not a good idea.

@cameron-p-m
Copy link
Contributor Author

@lance6716 I updated the code and added some tests for the GTID event.

@lance6716
Copy link
Collaborator

but it doesn't really work because of MariaDBs unsigned int64 vs MySQL signed int64

Sorry I didn't realised it before I recommend to put the SequenceNumber and LastCommitted into the interface. So maybe

type BinlogGTIDEvent interface {
	GTIDNext() (GTIDSet, error)
}

is the final version.

And IMO the BinlogGTIDEvent seems a bit unnatural because it only has one method 😂 Do you think it's better that the logic of "checking GTID dependency" can be encapsulated in this interface? like

type BinlogGTIDEvent interface {
...
	DependOn(previous BinlogGTIDEvent) bool
}

But in opposition to it, this library should only care about parsing and leave the dependency calculation to the upper application layer.

Do you have any ideas?

@cameron-p-m
Copy link
Contributor Author

cameron-p-m commented Sep 12, 2023

And IMO the BinlogGTIDEvent seems a bit unnatural because it only has one method 😂 Do you think it's better that the logic of "checking GTID dependency" can be encapsulated in this interface? like

type BinlogGTIDEvent interface {
...
DependOn(previous BinlogGTIDEvent) bool
}
But in opposition to it, this library should only care about parsing and leave the dependency calculation to the upper application layer.

Do you have any ideas?

I think the dependency calculation should be up to the application.

One issue with having it in Canal is that I don't think it could work in MariaDB and MySQL. The dependency calculation also changes if there is filtering, so I think the application has to decide anyways.

@lance6716 lance6716 merged commit 31e8e58 into go-mysql-org:master Sep 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants