-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Refine wait_timeout
override to be at cut-over only
#1406
Refine wait_timeout
override to be at cut-over only
#1406
Conversation
wait_timeout
override to be at cut-over only
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice work and in particular great analysis of this stalemate situation! This looks generally correct, but see inline comments:
- It break
gh-ost
for existing users - It adds variables where I think we can do without
- It adds code complexity which we can delegate to MySQL.
@shlomi-noach I'm curious about your feedback on one consequence of this change Following this PR, it is possible for the session holding the If understand this right, the lock-session hitting -initially-drop-old-table
Drop a possibly existing OLD table (remains from a previous run?) before beginning operation. Default is to panic and abort if such table exists Would |
No, actually. The The next cut-over attempt will first, before placing any locks, attempt to This should be safe. |
@shlomi-noach that makes sense (eventually)! Thanks for the validations and explanations |
@shlomi-noach / @meiji163: I believe I've addressed the PR suggestions and this is ready for another review 🙇 |
4d670bd
to
d0854d6
Compare
c660492
to
59db6fa
Compare
Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com>
Merging. @shlomi-noach let me know if I missed something and I'll make a follow-up PR 👍 |
Related issue: #1407
Description
This PR refines #1401 by overriding the session
wait_timeout
only where it is needed - at the cut-over time where an idle connection could lead to potentially-long table lock if thegh-ost
process (or host running it) "freezes"/"stalls" at the cut-over stageThe change (at cut-over only):
wait_timeout
is fetched (via an existingselect
that fetchedtime_zone
)wait_timeout
is set to be 3 x the lock-wait timeoutgh-ost
stalls with a still-active connection herewait_timeout
is restored to what it was set to pre-cut-overThe
--mysql-wait-timeout
flag added in #1401 is removed because it is no longer needed. No release has been cut since #1401, so this isn't necessarily a breaking changescript/cibuild
returns with no formatting errors, build errors or unit test errors.