mysqlctl: propagate remote shutdown timeout#20059
Conversation
Remote mysqlctl shutdown calls accepted a caller-provided timeout but dropped it when building the gRPC request, causing mysqlctld to fall back to its default shutdown timeout. This carries the timeout through the client interface and serializes it into the shutdown request. Signed-off-by: Mohamed Hamza <mhamza@fastmail.com>
Review ChecklistHello reviewers! 👋 Please follow this checklist when reviewing this Pull Request. General
Tests
Documentation
New flags
If a workflow is added or modified:
Backward compatibility
|
There was a problem hiding this comment.
Pull request overview
This PR fixes remote mysqlctl shutdown behavior by ensuring the caller-provided shutdown timeout is propagated over the mysqlctld gRPC boundary, instead of being dropped and replaced by mysqlctld’s default.
Changes:
- Extend the
MysqlctlClientShutdownmethod to accept ashutdownTimeout time.Duration. - Pass the shutdown timeout through
Mysqld.Shutdownwhen executing remotely via mysqlctld. - Serialize the timeout into
mysqlctlpb.ShutdownRequestusingprotoutil.DurationToProto.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| go/vt/mysqlctl/mysqld.go | Propagates shutdownTimeout into the remote mysqlctld shutdown call. |
| go/vt/mysqlctl/mysqlctlclient/interface.go | Updates the client interface to carry shutdownTimeout for remote shutdowns. |
| go/vt/mysqlctl/grpcmysqlctlclient/client.go | Adds MysqlShutdownTimeout to the gRPC ShutdownRequest payload. |
Codecov Report✅ All modified and coverable lines are covered by tests.
Additional details and impacted files@@ Coverage Diff @@
## main #20059 +/- ##
===========================================
- Coverage 69.67% 52.27% -17.40%
===========================================
Files 1614 46 -1568
Lines 216793 7128 -209665
===========================================
- Hits 151044 3726 -147318
+ Misses 65749 3402 -62347
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
mattlord
left a comment
There was a problem hiding this comment.
LGTM! Can we add some kind of test coverage for it?
Signed-off-by: Mohamed Hamza <mhamza@fastmail.com>
Signed-off-by: Mohamed Hamza <mhamza@fastmail.com> Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com>
…20074) Signed-off-by: Mohamed Hamza <mhamza@fastmail.com> Co-authored-by: vitess-bot[bot] <108069721+vitess-bot[bot]@users.noreply.github.com> Co-authored-by: Mohamed Hamza <mhamza@fastmail.com> Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Description
Remote mysqlctl shutdown calls accepted a caller-provided timeout but dropped it when building the gRPC request, causing mysqlctld to fall back to its default shutdown timeout. This carries the timeout through the client interface and serializes it into the shutdown request.
This is a small, isolated bug fix, so I'm suggesting to backport it to v23 and v24.
Related Issue(s)
Closes #20061
Checklist
Deployment Notes
AI Disclosure
Help from GPT 5.5.