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

multiregionccl: add a cold start latency test #92826

Merged
merged 4 commits into from
Jan 20, 2023

Conversation

ajwerner
Copy link
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
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 <[email protected]>
Co-authored-by: Ahmad Abedalqader <[email protected]>
Co-authored-by: Andrew Werner <[email protected]>
@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
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
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
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
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
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
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