backport https://github.com/vitessio/vitess/pull/16574 #185
backport https://github.com/vitessio/vitess/pull/16574 #185kamil-gwozdz wants to merge 2 commits into
Conversation
Signed-off-by: Harshit Gangal <harshit@planetscale.com>
I don't think we have any |
These are all artifacts from upstream. That's how they handle backporting new changes. For the most part, ignore everything in this PR 😅 . Just make sure CI passes |
|
"Reparent Old Vtctl - Upgrade Downgrade Testing" is failing but it's also failing here so I guess it's okay |
|
unit_race tests failed but they are also failing in the same way on other PRs so im gonna assume my change is fine |
There was a problem hiding this comment.
Pull Request Overview
This PR backports functionality related to handling the innodb_lock_wait_timeout system variable in Vitess, allowing it to be set and accessed at both session and global scopes.
- Moves
innodb_lock_wait_timeoutfrom read-only to settable system variables - Updates AST rewriting to properly handle global scope variables by not rewriting them
- Adds comprehensive test coverage for the new functionality
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| go/vt/vtgate/executor_set_test.go | Adds test cases for setting innodb_lock_wait_timeout at session and global scopes |
| go/vt/vtgate/executor_select_test.go | Adds comprehensive test for global and session variable behavior |
| go/vt/sysvars/sysvars.go | Moves innodb_lock_wait_timeout from read-only to settable variables list |
| go/vt/sqlparser/ast_rewriting.go | Updates variable rewriting logic to exclude global scope variables |
| go/test/endtoend/vtgate/reservedconn/sysvar_test.go | Adds end-to-end test for innodb_lock_wait_timeout functionality |
| go.mod | Adds indirect comment to prometheus/common dependency |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
You can also share your feedback on Copilot code review for a chance to win a $100 gift card. Take the survey.
| github.com/planetscale/vtprotobuf v0.5.0 | ||
| github.com/prometheus/client_golang v1.19.0 | ||
| github.com/prometheus/common v0.49.0 | ||
| github.com/prometheus/common v0.49.0 // indirect |
There was a problem hiding this comment.
Adding an // indirect comment to an existing dependency without actually making it indirect could be misleading. This change appears unrelated to the PR's purpose of backporting innodb_lock_wait_timeout functionality.
| github.com/prometheus/common v0.49.0 // indirect | |
| github.com/prometheus/common v0.49.0 |
|
@mhamza15 any chance you could help me roll this out? |
|
@kamil-gwozdz Rolling out any change across the fleet is somewhat blocked due to an issue that should be fixed by this upstream PR: vitessio#18530. We are planning on backporting it along with the v20 upgrade. I think our way forward is to include this change with v20 and roll it out along with the upgrade. The upgrade is targeted to complete by 9/12 (tracking issue). Does that sound good with y'all? |
|
v20 backport PR: #188 |
|
Sounds good, thanks. |
|
closing because it's gonna be backported to v20 instead - #185 (comment) |
Description
This PR backports vitessio#16574.
Related Issue(s)
https://github.com/github/issues/issues/17729
Checklist
Deployment Notes