Skip to content

roachtest: port rebalance/by-load/*/mixed-version to new framework#115559

Merged
craig[bot] merged 2 commits intocockroachdb:masterfrom
kvoli:231204.port-rebalance-load-mixed-version
Dec 8, 2023
Merged

roachtest: port rebalance/by-load/*/mixed-version to new framework#115559
craig[bot] merged 2 commits intocockroachdb:masterfrom
kvoli:231204.port-rebalance-load-mixed-version

Conversation

@kvoli
Copy link
Copy Markdown
Contributor

@kvoli kvoli commented Dec 4, 2023 •
edited
Loading

Port rebalance/by-load/leases/mixed-version and
rebalance/by-load/replicas/mixed-version to the new mixed-version
framework as described in #110528.

Part of: #110528
Resolves: #115134
Resolves: #115135
Release note: None

ts_util.go provides helper functions for querying the cluster
timeseries database over http. These methods previously made use of
httputil.PostJSON for requesting timeseries. This commit updates to
use httputil.PostProtobuf, in order to allow ignoring optional fields
when the server doesn't support them; namely TenantID.

Epic: none
Release note: None

@kvoli
roachtest: use protobuf instead of json for ts_util queries
dc8c484 
`ts_util.go` provides helper functions for querying the cluster
timeseries database over http. These methods previously made use of
`httputil.PostJSON` for requesting timeseries. This commit updates to
use `httputil.PostProtobuf`, in order to allow ignoring optional fields
when the server doesn't support them; namely `TenantID`.

Epic: none
Release note: None
@kvoli kvoli self-assigned this Dec 4, 2023
@cockroach-teamcity
Copy link
Copy Markdown
Member

cockroach-teamcity commented Dec 4, 2023

This change is Reviewable

@kvoli kvoli force-pushed the 231204.port-rebalance-load-mixed-version branch from ea43a13 to 6a3e45e Compare December 4, 2023 21:23
@kvoli kvoli added the backport-23.2.x PAST MAINTENANCE SUPPORT: 23.2 patch releases via ER request only label Dec 4, 2023
@kvoli kvoli changed the title roachtest: use protobuf instead of json for ts_util queries roachtest: port rebalance/by-load/*/mixed-version to new framework Dec 4, 2023
@kvoli
Copy link
Copy Markdown
Contributor Author

kvoli commented Dec 4, 2023

Passes remote run 
$ roachtest run 'rebalance/by-load/'
...
=== RUN   rebalance/by-load/replicas/mixed-version  [metrics: https://go.crdb.dev/roachtest-grafana/kvoli-1701725271/rebalance-by-load-replicas-mixed-version/1701725437558/1701727237558]
=== RUN   rebalance/by-load/leases  [metrics: https://go.crdb.dev/roachtest-grafana/kvoli-1701725271/rebalance-by-load-leases/1701725441471/1701727241471]
=== RUN   rebalance/by-load/leases/mixed-version  [metrics: https://go.crdb.dev/roachtest-grafana/kvoli-1701725271/rebalance-by-load-leases-mixed-version/1701725458329/1701727258329]
=== RUN   rebalance/by-load/replicas/ssds=2  [metrics: https://go.crdb.dev/roachtest-grafana/kvoli-1701725271/rebalance-by-load-replicas-ssds-2/1701725496171/1701727296171]
=== RUN   rebalance/by-load/replicas  [metrics: https://go.crdb.dev/roachtest-grafana/kvoli-1701725271/rebalance-by-load-replicas/1701725528188/1701727328188]
--- PASS: rebalance/by-load/leases (106.38s)
--- PASS: rebalance/by-load/replicas/ssds=2 (109.96s)
No work remaining; runWorker is bailing out...
--- PASS: rebalance/by-load/replicas (148.87s)
No work remaining; runWorker is bailing out...
No work remaining; runWorker is bailing out...
--- PASS: rebalance/by-load/leases/mixed-version (1222.12s)
No work remaining; runWorker is bailing out...
--- PASS: rebalance/by-load/replicas/mixed-version (1785.40s)
No work remaining; runWorker is bailing out...
PASS

@kvoli kvoli marked this pull request as ready for review December 4, 2023 22:44
@kvoli kvoli requested a review from a team as a code owner December 4, 2023 22:44
@kvoli kvoli requested review from herkolategan, nvb and srosenberg and removed request for a team December 4, 2023 22:44
@kvoli kvoli force-pushed the 231204.port-rebalance-load-mixed-version branch from 6a3e45e to bf85d3a Compare December 5, 2023 14:03
@kvoli kvoli mentioned this pull request Dec 5, 2023
Closed
nvb
nvb approved these changes Dec 5, 2023
View reviewed changes
Copy link
Copy Markdown
Contributor

@nvb nvb left a comment

Choose a reason for hiding this comment

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

:lgtm:

Reviewed 4 of 4 files at r1, 1 of 1 files at r2, all commit messages.
Reviewable status: :shipit: complete! 1 of 0 LGTMs obtained (waiting on @herkolategan, @kvoli, and @srosenberg)

-- commits line 16 at r3:
nit: missing space between Port and `rebalance/...

pkg/cmd/roachtest/tests/rebalance_load.go line 249 at r3 (raw file):

	require.NoError(t, WaitFor3XReplication(ctx, t, db))

	var m *errgroup.Group // see comment in version.go

What comment is this referring to?

@nvb nvb mentioned this pull request Dec 5, 2023
Merged
@kvoli
roachtest: port rebalance/by-load/*/mixed-version to new framework
55e2d6f 
Port `rebalance/by-load/leases/mixed-version` and
`rebalance/by-load/replicas/mixed-version` to the new mixed-version
framework as described in cockroachdb#110528.

Part of: cockroachdb#110528
Resolves: cockroachdb#115134
Resolves: cockroachdb#115135
Release note: None
@kvoli kvoli force-pushed the 231204.port-rebalance-load-mixed-version branch from bf85d3a to 55e2d6f Compare December 5, 2023 23:29
@kvoli kvoli requested a review from nvb December 5, 2023 23:30
kvoli
kvoli commented Dec 5, 2023
View reviewed changes
Copy link
Copy Markdown
Contributor Author

@kvoli kvoli left a comment

Choose a reason for hiding this comment

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

TYFTR

Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained (and 1 stale) (waiting on @herkolategan, @nvanbenschoten, and @srosenberg)

-- commits line 16 at r3:

Previously, nvanbenschoten (Nathan VanBenschoten) wrote…

nit: missing space between Port and `rebalance/...

Added a space here.

pkg/cmd/roachtest/tests/rebalance_load.go line 249 at r3 (raw file):

Previously, nvanbenschoten (Nathan VanBenschoten) wrote…

What comment is this referring to?

Comes from 094bf14.

Probably referring to deficiency of monitors for workload runners (5 years ago)?

cockroach/pkg/cmd/roachtest/version.go

Line 101 in 094bf14

// TODO(tschottdorf): `c.newMonitor` is far from suitable for tests that actually restart nodes

I removed the comment here, but will leave the errgroup to not shuffle around too much in this PR.

@kvoli
Copy link
Copy Markdown
Contributor Author

kvoli commented Dec 8, 2023

TYFTR

bors r=nvanbenschoten

@craig
Copy link
Copy Markdown
Contributor

craig bot commented Dec 8, 2023

Build succeeded:

@craig craig bot merged commit 8475571 into cockroachdb:master Dec 8, 2023
@blathers-crl
Copy link
Copy Markdown

blathers-crl bot commented Dec 8, 2023

Encountered an error creating backports. Some common things that can go wrong:

  1. The backport branch might have already existed.
  2. There was a merge conflict.
  3. The backport branch contained merge commits.

You might need to create your backport manually using the backport tool.

error creating merge commit from dc8c484 to blathers/backport-release-23.2-115559: POST https://api.github.com/repos/cockroachdb/cockroach/merges: 409 Merge conflict []

you may need to manually resolve merge conflicts with the backport tool.

Backport to branch 23.2.x failed. See errors above.

🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf.

@renatolabs
Copy link
Copy Markdown

renatolabs commented Dec 11, 2023

👋 backport reminder -- would be nice to have this in 23.2 and even in 23.1 if applicable.

@renatolabs renatolabs mentioned this pull request Dec 11, 2023
Closed
13 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@herkolategan herkolategan Awaiting requested review from herkolategan herkolategan was automatically assigned from cockroachdb/test-eng

@srosenberg srosenberg Awaiting requested review from srosenberg srosenberg was automatically assigned from cockroachdb/test-eng

@nvb nvb Awaiting requested review from nvb

Assignees

@kvoli kvoli

Labels

backport-23.2.x PAST MAINTENANCE SUPPORT: 23.2 patch releases via ER request only

Projects

None yet

Milestone

No milestone

4 participants

@kvoli @cockroach-teamcity @renatolabs @nvb