Skip to content
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

Make Maxwell compatible with MariaDB MaxScale #559

Merged
merged 4 commits into from
Feb 16, 2017
Merged

Make Maxwell compatible with MariaDB MaxScale #559

merged 4 commits into from
Feb 16, 2017

Conversation

adaszko
Copy link
Contributor

@adaszko adaszko commented Feb 8, 2017

There are 2 logical changes in the PR, each in its own commit:

  • Add separate, dedicated connection for retrieving database schema during Maxwell startup

This is required when replication_host option is set to MaxScale. MaxScale does not keep a copy of database schema. The commit introduces a new schema_{host,port,user,password} options that are to be configured with a location of a MySQL/MariaDB's node holding the schema.

  • Change default heartbeat interval from 0.5s to 1s

MaxScale is not capable of handling sub-second heartbeat intervals and requesting one, results (due to integer truncation) in heartbeat interval set to 0, which disables heartbeats altogether.

@adaszko
Copy link
Contributor Author

adaszko commented Feb 13, 2017

Hi, do these look acceptable for mainline Maxwell? The relevant changes on the MaxScale end have just been merged it looks like they will land in the next release.

@osheroff
Copy link
Collaborator

sorry for the lag, I've been travelling. will pull off some time to review soon.

@adaszko
Copy link
Contributor Author

adaszko commented Feb 13, 2017

Cool, thanks 🙂

@osheroff
Copy link
Collaborator

needs a test or two to exercise the case where the schema-server is different than the replication server. Otherwise looks good to merge.

@adaszko
Copy link
Contributor Author

adaszko commented Feb 14, 2017

Rebased and added a test case. Not sure what else to test there.

@osheroff
Copy link
Collaborator

looks like test failures were unrelated, kicked the build.

@osheroff osheroff merged commit a280690 into zendesk:master Feb 16, 2017
@osheroff
Copy link
Collaborator

(:+1)

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