Skip to content

multiregionccl: add a cold start latency test #92826

Merged
craig[bot] merged 4 commits intocockroachdb:masterfrom
ajwerner:ajwerner/latency-testing-demo
Jan 20, 2023
Merged

multiregionccl: add a cold start latency test #92826
craig[bot] merged 4 commits intocockroachdb:masterfrom
ajwerner:ajwerner/latency-testing-demo

Conversation

@ajwerner
Copy link
Copy Markdown
Contributor

@ajwerner ajwerner commented Dec 1, 2022

This commit adds a test which creates an MR serverless cluster and then
boots the sql pods in each region while disallowing connectivity to other
regions. It also simulates latency to make sure the routing logic works
and to provide a somewhat realistic picture of what to expect.

Epic: CRDB-18596

Release note: None

@cockroach-teamcity
Copy link
Copy Markdown
Member

This change is Reviewable

@ajwerner ajwerner force-pushed the ajwerner/latency-testing-demo branch from 3085659 to 0f33e97 Compare December 1, 2022 19:12
ajwerner added a commit to ajwerner/cockroach that referenced this pull request Dec 9, 2022
This is an oversight from cockroachdb#92588. Testing will come in cockroachdb#92826.

Epic: CRDB-18596

Release note: None
@ajwerner ajwerner force-pushed the ajwerner/latency-testing-demo branch 2 times, most recently from 972c163 to d3874fb Compare December 9, 2022 20:23
craig bot pushed a commit that referenced this pull request Dec 10, 2022
93294: dev: do not log noisy bep flag r=rickystewart a=healthy-pod

Release note: None
Epic: none

93342: sql: partition lease table when optimizing system database for MR r=ajwerner a=ajwerner

This is an oversight from #92588. Testing will come in #92826.

Epic: CRDB-18596

Release note: None

Co-authored-by: healthy-pod <ahmad@cockroachlabs.com>
Co-authored-by: Ahmad Abedalqader <ahmad@cockroachlabs.com>
Co-authored-by: Andrew Werner <awerner32@gmail.com>
@ajwerner ajwerner force-pushed the ajwerner/latency-testing-demo branch from d3874fb to f70079a Compare December 12, 2022 17:46
@ajwerner ajwerner changed the title [dnm] multiregionccl: add a cold start latency test multiregionccl: add a cold start latency test Dec 12, 2022
@ajwerner ajwerner marked this pull request as ready for review December 12, 2022 17:47
@ajwerner ajwerner requested review from a team as code owners December 12, 2022 17:47
@ajwerner ajwerner requested a review from jeffswenson January 16, 2023 22:41
Copy link
Copy Markdown
Collaborator

@fqazi fqazi left a comment

Choose a reason for hiding this comment

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

Reviewed 2 of 2 files at r1, 3 of 3 files at r2, 1 of 1 files at r3, 3 of 3 files at r4, all commit messages.
Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained (waiting on @jeffswenson)

@ajwerner ajwerner force-pushed the ajwerner/latency-testing-demo branch from f70079a to e926308 Compare January 19, 2023 17:25
Copy link
Copy Markdown
Collaborator

@jeffswenson jeffswenson left a comment

Choose a reason for hiding this comment

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

LGTM

)
localities := makeLocalities(regionLatencies, numNodes, numAZsPerRegion)
regions := make([]string, len(localities))
{
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

nit: is the extra block wrapping l.Find intentional?

t.Log("pre running test to allocate instance IDs")
t.Log("result", localities, runAllTests())
t.Log("running test with no connectivity from sql pods to remote regions")
blockCrossRegionTenantAccess.Store(true)
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Blocking cross-region network traffic is a neat check.

@ajwerner ajwerner force-pushed the ajwerner/latency-testing-demo branch from e926308 to e3e4bdc Compare January 20, 2023 19:15
This commit adds a test which creates an MR serverless cluster and then
boots the sql pods in each region while disallowing connectivity to other
regions. It also simulates latency to make sure the routing logic works
and to provide a somewhat realistic picture of what to expect.

Release note: None
@ajwerner ajwerner force-pushed the ajwerner/latency-testing-demo branch from e3e4bdc to 4c59d79 Compare January 20, 2023 19:24
Copy link
Copy Markdown
Contributor Author

@ajwerner ajwerner left a comment

Choose a reason for hiding this comment

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

TFTR!

bors r+

Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained (waiting on @fqazi)


pkg/ccl/multiregionccl/cold_start_latency_test.go line 71 at r8 (raw file):

Previously, JeffSwenson (Jeff Swenson) wrote…

nit: is the extra block wrapping l.Find intentional?

Done.

@craig craig bot merged commit 1b79102 into cockroachdb:master Jan 20, 2023
@craig
Copy link
Copy Markdown
Contributor

craig bot commented Jan 20, 2023

Build succeeded:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants