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

Gtid tracking support #151

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

chris-brundage
Copy link

Pull Request (PR) description

ProxySQL 2.0.1+ includes a number of schema changes to support GTID tracking for casual consistency reads. The proxy_mysql_server and proxy_mysql_server_no_hostgroup types currently are not aware of the new gtid_port column in the mysql_servers table.

This PR adds 1) a function to the Proxysql parent provider class that indicates if the installed ProxySQL version has GTID tracking support (has_gtid_tracking?), which is based off the proxysql_version fact, and 2) support for gtid_port on Proxysql::Server types.

The changes are also backwards compatible. Older and unknown versions ignore the gtid_port parameter.

This Pull Request (PR) fixes the following issues

n/a

Add a function to the parent provider that indicates if the installed version of ProxySQL has support for GTID tracking (2.0.1+). This allows providers to implement support for the 2.0.1+ schema changes without breaking backwards compatibility.
Add a new optional parameter to the proxy_mysql_server and proxy_mysql_server_no_hostgroup types called "gtid_port". In order to maintain backward compatibility, the associated providers ignore the parameter if the ProxySQL verison is too old to support GTID tracking, or if the proxysql_version fact is not set.
@vox-pupuli-tasks
Copy link

Dear @chris-brundage, thanks for the PR!

This is Vox Pupuli Tasks, your friendly Vox Pupuli Github Bot. I noticed that your pull request has CI failures. Can you please have a look at the failing CI jobs?
If you need any help, you can reach out to us on our IRC channel voxpupuli on Freenode or our Slack channel voxpupuli at slack.puppet.com.
You can find my sourcecode at voxpupuli/vox-pupuli-tasks

Copy link
Member

@mcrauwel mcrauwel left a comment

Choose a reason for hiding this comment

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

After you fix the failing tests we'll need to implement a test to validate that things would not break on version 1.4.x and a test to validate that supplying a GTID-port value will actually change the configuration correctly...

@@ -57,6 +64,7 @@ def create
_name = @resource[:name]
hostname = @resource.value(:hostname)
port = @resource.value(:port) || 3306
gtid_port = @resource.value(:gtid_port)
Copy link
Member

Choose a reason for hiding this comment

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

this will need a default value to fix failing tests

@@ -57,6 +64,7 @@ def create
_name = @resource[:name]
hostname = @resource.value(:hostname)
port = @resource.value(:port) || 3306
gtid_port = @resource.value(:gtid_port)
Copy link
Member

Choose a reason for hiding this comment

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

this will need a default value to fix failing tests

@vox-pupuli-tasks
Copy link

Dear @chris-brundage, thanks for the PR!

This is Vox Pupuli Tasks, your friendly Vox Pupuli GitHub Bot. I noticed that your pull request contains merge conflict. Can you please rebase?

You can find my sourcecode at voxpupuli/vox-pupuli-tasks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants