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.

Sign up for GitHub

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

Jump to bottom

Backport: set vreplication net read and net write timeout session vars to high values #155

Merged
merged 1 commit into from
Apr 9, 2024
Merged

Backport: set vreplication net read and net write timeout session vars to high values #155

merged 1 commit into from
Apr 9, 2024
+55 −0

Conversation

pawandubey
Copy link

Backport of vitessio@6953390
Redo of #129 onto the newest release.
Part of https://github.com/Shopify/vitess-project/issues/623

Helps with failure of schema changes using vitess strategy due to lower than desired timeout values.

⚠️ Does not contain bits from the original PR that reference features that were introduced in releases later than 15. (e.g. change in tablestreamer.go introduced in vitessio@04ad6ec, and in rpc_vreplication_test.go which was introduced in vitessio@20e4768).

@olyazavr @pawandubey
set vreplication net read and net write timeout session vars to high …
71d548c 
…values (vitessio#14203)

Signed-off-by: Olga Shestopalova <[email protected]>
Co-authored-by: Olga Shestopalova <[email protected]>
@pawandubey pawandubey self-assigned this Apr 5, 2024
@pawandubey pawandubey marked this pull request as ready for review April 5, 2024 19:21
pawandubey
pawandubey commented Apr 5, 2024
View reviewed changes
go/vt/vttablet/flags.go
)

func init() {
servenv.OnParseFor("vttablet", registerFlags)
Copy link
Author

@pawandubey pawandubey Apr 5, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The previous attempt also passed these flags to vtcombo which is used in vttestserver so would be useful in dev and tests, but given that upstream PR does not add it, I am wary of adding to it. Also unsure if the schema migration issue will crop up in dev anyway.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

makes sense to leave it out.

@pawandubey
Copy link
Author

Two checks are failing for non-code reasons:

  1. Unit Test (mariadb103) / test (pull_request) is failing because one of the repository links no longer exists. The rackspace mirror now only hosts mariadb 10.4 and later, and not 10.3: https://mirror.rackspace.com/mariadb/repo/. I think it's okay to ignore this as we don't use mariadb.
  2. Static Code Checks, Etc. / Static Code Checks Etc (pull_request) is failing because an API token for the static code scanning platform FOSSA was changed and this version of the code does not have it. I think this is okay to ignore also?

shivnagarajan
shivnagarajan approved these changes Apr 5, 2024
View reviewed changes
go/vt/vttablet/flags.go
)

func init() {
servenv.OnParseFor("vttablet", registerFlags)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

makes sense to leave it out.

shanth96
shanth96 approved these changes Apr 5, 2024
View reviewed changes
Copy link

@shanth96 shanth96 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we also want to port the tests?

@pawandubey
Copy link
Author

Do we also want to port the tests?

@shanth96 the tests were added to a file that does not yet exist in v15. I tried to note that in the PR description. I did not want to introduce the file here since the tests are rather simplistic? Happy to be convinced otherwise.

brendar
brendar approved these changes Apr 5, 2024
View reviewed changes
Copy link

@brendar brendar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@pawandubey pawandubey merged commit 84ea974 into v15.0.3-shopify-11 Apr 9, 2024
220 of 228 checks passed
shivnagarajan pushed a commit that referenced this pull request May 8, 2024
@pawandubey @shivnagarajan
Merge pull request #155 from Shopify/candidate-v15.0.3-shopify-11
dde1210 
Backport: set vreplication net read and net write timeout session vars to high values
(cherry picked from commit 84ea974)
(cherry picked from commit 5cc2dfc)
shivnagarajan pushed a commit that referenced this pull request May 14, 2024
@pawandubey @shivnagarajan
Merge pull request #155 from Shopify/candidate-v15.0.3-shopify-11
fabdae5 
Backport: set vreplication net read and net write timeout session vars to high values
(cherry picked from commit 84ea974)
(cherry picked from commit 5cc2dfc)
(cherry picked from commit dde1210)
shivnagarajan pushed a commit that referenced this pull request May 19, 2024
@pawandubey @shivnagarajan
Merge pull request #155 from Shopify/candidate-v15.0.3-shopify-11
e05a814 
Backport: set vreplication net read and net write timeout session vars to high values
(cherry picked from commit 84ea974)
(cherry picked from commit 5cc2dfc)
(cherry picked from commit dde1210)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@shanth96 shanth96 shanth96 approved these changes

@shivnagarajan shivnagarajan shivnagarajan approved these changes

@austenLacy austenLacy austenLacy approved these changes

@brendar brendar brendar approved these changes

Assignees

@pawandubey pawandubey

Labels
Available: 19.0.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@pawandubey @brendar @austenLacy @shivnagarajan @shanth96 @olyazavr