[keyserver] Limit mysqldump backup queries to 0.5 MiB
ClosedPublic
Actions

Authored by ashoat on Jul 26 2022, 4:46 AM.

Details

Reviewers

atul
tomek

Commits

rCOMMbdd4a6e23124: [keyserver] Limit mysqldump backup queries to 0.5 MiB

Summary

I was seeing a very strange issue where restores of mysqldump backups would fail to correctly restore two specific rows in the notifications table, causing the delivery column to become corrupted.

After extensive testing spanning multiple days, I found two ways to avoid the problem:

This fix, wherein we limit the number of rows in each INSERT.
Deleting the SET FOREIGN_KEY_CHECKS=0 line that gets automatically inserted at the top of the backup file by mysqldump.

I can't tell why the second option works and there's no way to configure mysqldump to prevent it from spitting out that line. We can remove it after-the-fact but that would be more work.

On the other hand, this solution is simple and "just works".

Test Plan

I patched the change on my production keyserver and generated a backup from there
I downloaded the backup to my laptop and restored it
I then generated a second backup (with the change) on my laptop
Finally I restored the second backup on my laptop, confirming there was no corruption and that everything worked

Diff Detail

Repository

rCOMM Comm

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

ashoat created this revision.Jul 26 2022, 4:46 AM

Herald added subscribers: • abosh, • adrian. · View Herald TranscriptJul 26 2022, 4:46 AM

ashoat edited the summary of this revision. (Show Details)Jul 26 2022, 4:49 AM

ashoat removed a parent revision: D4641: [keyserver] Use mysqldump from MariaDB 10.8 instead of MySQL 8.

Harbormaster completed remote builds in B10880: Diff 14958.Jul 26 2022, 4:55 AM

ashoat requested review of this revision.Jul 26 2022, 4:55 AM

Not sure if this doc https://dev.mysql.com/doc/refman/5.7/en/server-system-variables.html#sysvar_net_buffer_length applies to mysqldump, but if that's the case then

Each client thread is associated with a connection buffer and result buffer. Both begin with a size given by net_buffer_length but are dynamically enlarged up to max_allowed_packet bytes as needed.

So if the intention is to

Limit mysqldump backup queries to 0.5 MiB

Then it might be not what actually happens, as according to the doc, the buffers

are dynamically enlarged up to max_allowed_packet bytes as needed

Not going to block this diff, as it seems to solve the problem, but the config here is confusing.

keyserver/src/cron/backups.js
100 ↗	(On Diff #14958)	It is not exactly `0.5MB` - it should probably be `524288 = 2^19`.

This revision is now accepted and ready to land.Jul 26 2022, 6:54 AM

Agree the config is confusing. However, I can confirm that this diff about halves the size of each INSERT line in the backup. Before this diff, there were 82 INSERT queries to populate the notifications table. After this diff, there were 163.

keyserver/src/cron/backups.js
100 ↗	(On Diff #14958)	Ah, interesting. To come up with this number, I called `mysqldump --help` from the production Node Docker image (which is `mysqldump` from MySQL 8.0), which printed a value of 1046528 for `net-buffer-length`. I then halved the number. The same number (1046528) seems to be used as a default by MariaDB. I'm not sure why they have that number precisely, but this change works and I'd prefer to avoid testing again (it takes a while to test this).