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

slack-vitess-r15.0.5: fix races in Unit Test (Race) CI, fix "old" reparent CIs #356

Merged

Conversation

timvaillancourt
Copy link
Member

@timvaillancourt timvaillancourt commented May 19, 2024

Description

This PR fixes a race introduced in PR #297. This patch added a *api.Config as a global var and it turns out this contains an *http.Transport that will be held open due to global var scope - this can cause races in tests that leave idle connections open

Secondly, a race in go/vt/servenv is causing race CI problems. I partially pulled-in a flaky test fix from vitessio#11520 (see "Incidental flaky tests" in summary) to address this

This PR:

  • Stops creating the *api.Config as a global var, instead the config is created every .NewServer() call
  • Make --topo_consul_max_conns_per_host default to 250 now that this value is rolled out everywhere (unrelated to race) and the default of 0 (unlimited) is dangerous
  • Ensures the vttestserver calls defer topoServer.Close()
  • Adds a lock to .HandleFunc(...) from go/vt/servenv (taken from vttablet sidecar schema:use schemadiff to reach desired schema on tablet init replacing the withDDL-based approach vitessio/vitess#11520)
  • Add mutex to .IsFlagProvided(...) from go/internal/flag
  • Skips the TestCrossCellDurability test when testing "old" vtctld/vttablet (ie: v14) reparents
    • Env var SKIPTESTCROSSCELLDURABILITY=1 to skip
    • This test is really from v16 (a backport)
    • These tests assume Durability Policies are already controlling semi-sync. On v14 that's not the case

Related Issue(s)

Checklist

  • "Backport to:" labels have been added if this change should be back-ported
  • Tests were added or are not required
  • Did the new or modified tests pass consistently locally and on the CI
  • Documentation was added or is not required

Deployment Notes

Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com>
Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com>
Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com>
Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com>
Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com>
Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com>
Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com>
Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com>
Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com>
Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com>
Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com>
@timvaillancourt timvaillancourt requested a review from a team as a code owner May 19, 2024 20:59
@timvaillancourt timvaillancourt added bug Something isn't working v15 labels May 19, 2024
@github-actions github-actions bot added this to the v15.0.5 milestone May 19, 2024
Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com>
@timvaillancourt timvaillancourt changed the title slack-vitess-r15.0.5: fix race in consultopo patch slack-vitess-r15.0.5: fix races in Unit Test (Race) May 19, 2024
@timvaillancourt timvaillancourt changed the title slack-vitess-r15.0.5: fix races in Unit Test (Race) slack-vitess-r15.0.5: fix races in Unit Test (Race) CI May 19, 2024
Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com>
Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com>
Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com>
Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com>
@timvaillancourt timvaillancourt changed the title slack-vitess-r15.0.5: fix races in Unit Test (Race) CI slack-vitess-r15.0.5: fix races in Unit Test (Race) CI, fix "old" reparent CIs May 20, 2024
Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com>
@timvaillancourt timvaillancourt merged commit a2a622a into slack-vitess-r15.0.5 May 20, 2024
192 of 194 checks passed
@timvaillancourt timvaillancourt deleted the fix-consultopo-race-slack-vitess-r15.0.5 branch May 20, 2024 16:49
timvaillancourt added a commit that referenced this pull request Jul 9, 2024
timvaillancourt added a commit that referenced this pull request Jul 9, 2024
* Make `Durabler` interface methods public (vitessio#15548)

Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com>
Signed-off-by: Manan Gupta <manan@planetscale.com>
Co-authored-by: Manan Gupta <manan@planetscale.com>

* Load `--grpc_auth_static_client_creds` file once (vitessio#15030)

Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com>

* Limit concurrent creation of healthcheck gRPC connections (vitessio#15053)

Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com>

* Filter by keyspace earlier in `tabletgateway`s `WaitForTablets(...)` (vitessio#15347)

Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com>

* Use slack-15.0 as previous release

Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com>

* empty commit

Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com>

* force ci to run

* Update GH Action runners

Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com>

* test templates

Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com>

* set GH access token in build

Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com>

* Fix reparent old tests

Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com>

* Remove CIs we don't need

Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com>

* Remove CIs we don't need again

Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com>

* Add private repo setup to upgrade_downgrade_test_backups_e2e.yml

Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com>

* Add private repo setup to more CI

Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com>

* remove CI skip logic for upstream stuff

Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com>

* CODEOWNERS

Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com>

* [release-19.0] Add timeout to all the contexts used for RPC calls in vtorc (vitessio#15991) (vitessio#16103)

Signed-off-by: Manan Gupta <manan@planetscale.com>

* `slack-vitess-r15.0.5`: forward-port consul topo limits PR #111 (#297)

* `slack-vitess-r14.0.5`: allow conn overrides in consul topo (#111)

* `slack-vitess-r14.0.5`: allow conn overrides in consul topo

Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com>

* fix e2e test

Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com>

---------

Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com>

* Update flags tests that didn't exist in v14

Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com>

---------

Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com>

* update vtcombo e2e

Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com>

* Fix err with installing percona-xtrabackup-24

Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com>

* `slack-vitess-r15.0.5`: fix races in `Unit Test (Race)` CI, fix "old" reparent CIs (#356)

* update vtcombo e2e test

Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com>

* Fix bad merge conflict fix

Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com>

* go mod tidy

Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com>

* update vtcombo e2e test again

Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com>

* [release-19.0] Upgrade the Golang version to `go1.22.5` (vitessio#16322)

Signed-off-by: GitHub <noreply@github.com>
Signed-off-by: Florent Poinsard <florent.poinsard@outlook.fr>
Co-authored-by: frouioui <frouioui@users.noreply.github.com>
Co-authored-by: Florent Poinsard <florent.poinsard@outlook.fr>

* merge conflict fixes

Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com>

* make vtadmin_web_proto_types

Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com>

---------

Signed-off-by: Tim Vaillancourt <tim@timvaillancourt.com>
Signed-off-by: Manan Gupta <manan@planetscale.com>
Signed-off-by: GitHub <noreply@github.com>
Signed-off-by: Florent Poinsard <florent.poinsard@outlook.fr>
Co-authored-by: Manan Gupta <manan@planetscale.com>
Co-authored-by: Manan Gupta <35839558+GuptaManan100@users.noreply.github.com>
Co-authored-by: vitess-bot <139342327+vitess-bot@users.noreply.github.com>
Co-authored-by: frouioui <frouioui@users.noreply.github.com>
Co-authored-by: Florent Poinsard <florent.poinsard@outlook.fr>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working v15
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants