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

set proxysql weight to 1 for hosts named 'reporting' #43262

Merged
merged 4 commits into from Jan 13, 2022

Conversation

cat5inthecradle
Copy link
Contributor

@cat5inthecradle cat5inthecradle commented Oct 28, 2021

Original logic was inferring that any Aurora instance in host group 3 was necessarily the 'reporting' instance, and therefore should be weighted at '1' to avoid traffic. However, due to some issue with ProxySQL not always identifying the instance, it might not see it in host group 3, and therefore weight it at '10000000' and allowing it to take on half of our traffic.

This solution adds to that rule by also identifying instances with a name LIKE 'reporting%', allowing the correct weighting to occur even if above issue occurs.

Hardcoding the name is a bit of a code smell, but it's at least straightforward. Very interested in suggestions on how the comments could be clearer here.

Links

Testing story

The SQL logic has been manually tested with a local SQLite database. Our plan is to test this with a detatched EC2 instance in AWS to see it working on prod-like resource.

Deployment strategy

...

PR Checklist:

  • Tests provide adequate coverage
  • Privacy and Security impacts have been assessed
  • Code is well-commented
  • New features are translatable or updates will not break translations
  • Relevant documentation has been added or updated
  • User impact is well-understood and desirable
  • Pull Request is labeled appropriately
  • Follow-up work items (including potential tech debt) are tracked and linked

Copy link
Contributor

@Hamms Hamms left a comment

Choose a reason for hiding this comment

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

Do we use these hostgroups for anything other than this weighting? I'm inclined to say we should remove the concept of "hostgroup three" entirely, and ONLY identify the reporting instance by name.

@cat5inthecradle
Copy link
Contributor Author

@sureshc @Hamms I pushed an updated version and added a comment for tangent issue.

I've also got some notes from troubleshooting this that may come in handy in the future. Thoughts on where to stash them or other ways to make this easier in the future?


update_servers.sql Notes

Seting up a local sqlite instance

docker run --rm -t keinos/sqlite3
CREATE TABLE `mysql_servers` (
  `hostgroup_id` int(11) DEFAULT NULL,
  `hostname` varchar(45) DEFAULT NULL,
  `port` int(11) DEFAULT NULL,
  `status` varchar(45) DEFAULT NULL,
  `weight` int(11) DEFAULT NULL,
  PRIMARY KEY (`hostgroup_id`, `hostname`));

Useful SQLite Queries

Delete the table

DELETE FROM mysql_servers;

Populate the table with a prod-like configuration

INSERT INTO mysql_servers
VALUES
  (0, 'writer', 3306, 'fine', 10000000),
  (1, 'writer', 3306, 'fine', 1),
  (1, 'reader', 3306, 'fine', 10000000),
  (1, 'reporting', 3306, 'fine', 1),
  (2, 'writer', 3306, 'fine', 10000000),
  (2, 'reader', 3306, 'fine', 1),
  (2, 'reporting', 3306, 'fine', 1),
  (3, 'reporting', 3306, 'fine', 1);

Inspect at the table

SELECT * FROM mysql_servers;

Weighting Logic

Note: The real logic is in update_servers.sql, this is just a copy.

-- Set HG2 copied from HG1.
REPLACE INTO `mysql_servers` (hostgroup_id, hostname, port, status, weight)
    SELECT 2, hostname, port, status, weight FROM `mysql_servers` WHERE `hostgroup_id` = 1;

-- Update weights on servers.
UPDATE `mysql_servers` SET `weight` = CASE
  WHEN ( -- If row is a writer (hostname in HG0)
    `hostname` IN (SELECT `hostname` FROM `mysql_servers` WHERE (`hostgroup_id` = 0))
  ) THEN -- Set writer weight low for HG1, high for HG2.
    CASE WHEN (`hostgroup_id` = 1) THEN 1 ELSE 10000000 END
  WHEN ( -- If row is a reporting-reader (hostname in HG3 or named 'reporting*')
    `hostname` LIKE 'reporting%'
    OR `hostname` IN (SELECT `hostname` FROM `mysql_servers` WHERE (`hostgroup_id` = 3))
  ) THEN -- Set reporting-reader weight low for all HG.
    1
  ELSE -- If row is a reader (not a writer or reporting-reader)
    -- Set reader weight high for HG1, low for HG2.
    CASE WHEN (`hostgroup_id` = 1) THEN 10000000 ELSE 1 END
  END;

@cat5inthecradle cat5inthecradle requested review from a team and removed request for sureshc October 29, 2021 22:03
@Hamms
Copy link
Contributor

Hamms commented Oct 29, 2021

My general preference is for documentation to live as close to the code as possible; in this case, I think it'd make sense to add this to the existing comment in the file itself!

@cat5inthecradle
Copy link
Contributor Author

cat5inthecradle commented Oct 30, 2021 via email

@Hamms
Copy link
Contributor

Hamms commented Nov 1, 2021

That sounds good to me! I'm a little worried about discoverability, so I'd just recommend we make the references we make to it in the comments be real obvious. But otherwise, documentation being in markdown and in the repository seems like exactly what we should be doing.

@sureshc sureshc requested a review from wjordan November 1, 2021 17:29
@cat5inthecradle
Copy link
Contributor Author

Tested this in production on 2022-01-13, did not cause any problems when running normally. Merging.

@cat5inthecradle cat5inthecradle merged commit 9ff886f into staging Jan 13, 2022
@cat5inthecradle cat5inthecradle deleted the inf-503-fix-proxysql-weight-logic branch January 13, 2022 16:14
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.

None yet

3 participants