Skip to content

fix canal.GetMasterPos() for mariadb #1030

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 5 commits into from
May 6, 2025

Conversation

alarbada
Copy link
Contributor

@alarbada alarbada commented May 2, 2025

Adds support for mariadb for the method canal.GetMasterPos().

Fixes #1029

@dveeden dveeden requested review from lance6716 and dveeden May 4, 2025 09:52
Copy link
Collaborator

@dveeden dveeden left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might be good to add some references to release notes and/or documentation.

This could be made into a function that takes a flavor and version and returns the query string. That would make it easier to add a unit test for this.

@alarbada alarbada requested review from dveeden and lance6716 May 5, 2025 13:32
@lance6716 lance6716 merged commit 61638c4 into go-mysql-org:master May 6, 2025
17 checks passed
@alarbada
Copy link
Contributor Author

alarbada commented May 6, 2025

@lance6716 thanks for accepting my pull request :)
I do need this fix to add mariadb support on the mysql conduit connector I'm working on. When do you think you could release this little fix, in a v1.12.1 or on the upcoming v1.13.0?

@dveeden
Copy link
Collaborator

dveeden commented May 6, 2025

@alarbada I assume you've tested with go mod edit -replace ... to make sure this is the only issue that's needed to make this work with recent MariaDB releases?

@dveeden
Copy link
Collaborator

dveeden commented May 6, 2025

I've created #1032

@alarbada
Copy link
Contributor Author

alarbada commented May 6, 2025

@alarbada I assume you've tested with go mod edit -replace ... to make sure this is the only issue that's needed to make this work with recent MariaDB releases?

Not really, after backporting the fix to my repo I got further problems with the replication.RowsEvent. For some reason the LogPos from EventHeader is 0, I'm debugging why.

I will add an issue (hopefully a PR) once I figure out exactly why is this happening.

@dveeden
Copy link
Collaborator

dveeden commented May 6, 2025

@alarbada OK, Please let me know once you figured this out. I'll wait with releasing a new version.

@alarbada alarbada deleted the mariadb-masterpos-fix branch May 6, 2025 12:36
@serprex
Copy link
Contributor

serprex commented May 7, 2025

@dveeden as a point of comparison, we're working with MariaDB on PeerDB, & had this fix in our logic already: https://github.com/PeerDB-io/peerdb/blob/84fe9ce286a8c8cc03a6369e4b7ce4b2f7505d1c/flow/connectors/mysql/mysql.go#L305

Link also shows references for versions where language shifted

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.

func (c *canal.Canal) GetMasterPos() always errors when using the mariadb flavor on latest mariadb v11
4 participants