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

All MySQL DBs limited to max 3 concurrent/idle connections #15 #931

Merged
merged 25 commits into from
May 27, 2021

Conversation

shlomi-noach
Copy link
Contributor

Resubmission of openark#15 from downstream.

Issue reported privately by community user. In some scenario (user had semi-sync + long row data) gh-ost connections piled up to MAX_USER_CONNECTIONS. This PR puts a hard limit (constant 3) on all pools. There's several of those (inspector, applier, throttler, and more), but each is limited to to 3 connections which at least does not cause a connection pileup on the MySQL server(s).

  • contributed code is using same conventions as original code
  • script/cibuild returns with no formatting errors, build errors or unit test errors.

Support a complete ALTER TABLE statement in --alter
Initial commit: towards setting up a test suite

Signed-off-by: Shlomi Noach <[email protected]>
…original table, applying AUTO_INCREMENT value onto ghost table if applicable and user has not specified AUTO_INCREMENT in alter statement
Signed-off-by: Shlomi Noach <[email protected]>
Signed-off-by: Shlomi Noach <[email protected]>
Copying AUTO_INCREMENT value to ghost table
Generated column as part of UNIQUE (or PRIMARY) KEY
Cut-over should wait for heartbeat lag to be low enough to succeed
@timvaillancourt
Copy link
Collaborator

@shlomi-noach 👋 that's a good idea, thanks for the PR 👍

I'm seeing potentially unrelated changes vs openark#15 in this PR. Can you confirm?

@shlomi-noach
Copy link
Contributor Author

I'm seeing potentially unrelated changes vs openark#15 in this PR. Can you confirm?

@timvaillancourt yeah, that's due to #911 and #919 not merged yet, I've continued on top of these. I can sanitize the relevant changes to this PR and re-submit (but hoping it would only be a matter of tim etill #911 and #919 are merged).

@timvaillancourt
Copy link
Collaborator

I'm seeing potentially unrelated changes vs openark#15 in this PR. Can you confirm?

@timvaillancourt yeah, that's due to #911 and #919 not merged yet, I've continued on top of these. I can sanitize the relevant changes to this PR and re-submit (but hoping it would only be a matter of tim etill #911 and #919 are merged).

@shlomi-noach ahh thanks. No problem, I can merge those PRs together eventually 👍

@timvaillancourt timvaillancourt self-requested a review May 6, 2021 12:13
@shlomi-noach
Copy link
Contributor Author

branch updated after recent merge of downstream contribution.

@timvaillancourt
Copy link
Collaborator

This low-risk changed has passed many tests, merging it 👍

Thanks @shlomi-noach 🎉

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.

2 participants