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

kvflowcontrol: add simulation unit test equivalent to kvflowsimulator #130186

Closed
Tracked by #129276
kvoli opened this issue Sep 5, 2024 · 0 comments · Fixed by #130446
Closed
Tracked by #129276

kvflowcontrol: add simulation unit test equivalent to kvflowsimulator #130186

kvoli opened this issue Sep 5, 2024 · 0 comments · Fixed by #130446
Assignees
@kvoli
Labels
A-replication-admission-control-v2 Related to introduction of replication AC v2 C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) T-kv KV Team

Comments

@kvoli
Copy link
Collaborator

kvoli commented Sep 5, 2024 •
edited by exalate-issue-sync bot
Loading

Add a unit test which has similar coverage as kvflowsimulator for rac2:

cockroach/pkg/kv/kvserver/kvflowcontrol/kvflowsimulator/simulation_test.go

Lines 43 to 47 in d3e15d7

// TestUsingSimulation is a data-driven test for kvflowcontrol.Controller and
// kvflowcontrol.Handle. It gives package authors a way to understand how flow
// tokens are maintained for individual replication streams, how write bandwidth
// is shaped by these tokens, and how requests queue/dequeue internally. We
// provide the following syntax:

Jira issue: CRDB-41914

Epic CRDB-37515
@kvoli kvoli mentioned this issue Sep 5, 2024
Closed
3 tasks
@blathers-crl blathers-crl bot added the X-blathers-untriaged blathers was unable to find an owner label Sep 5, 2024
@kvoli kvoli changed the title Simulation unit test equivalent to the tests in pkg/.../kvflowsimulator kvflowcontrol: add simulation unit test equivalent to kvflowsimulator Sep 5, 2024
@kvoli kvoli added C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) T-kv KV Team A-replication-admission-control-v2 Related to introduction of replication AC v2 and removed X-blathers-untriaged blathers was unable to find an owner labels Sep 5, 2024
@cockroachdb cockroachdb deleted a comment from blathers-crl bot Sep 5, 2024
@kvoli kvoli self-assigned this Sep 10, 2024
@kvoli kvoli mentioned this issue Sep 10, 2024
Merged
kvoli added a commit to kvoli/cockroach that referenced this issue Sep 13, 2024
@kvoli
rac2: add simulation test
7904428 
...

Resolves: cockroachdb#130186
Release note: None
kvoli added a commit to kvoli/cockroach that referenced this issue Sep 13, 2024
@kvoli
rac2: add kvflowsimulator equivalent simulation test
c95cf4e 
Add a new simulation test, `TestUsingSimulation`. This test mirrors
`kvflowsimulator.TestUsingSimulation` and its existing datadriven tests
cases.

When comparing the output from each, no behavioral differences show up,
only intentionally different metric names.

Note that like the `kvflowsimulator.TestUsingSimulation`, the simulation
test needs to be deterministic and therefore only uses a single
goroutine, to avoid goroutine scheduling causing non-determinism. As
such, helper methods have been added to facilitate waiting for available
tokens directly on a stream's token counter and on a range in a
non-blocking manner (`testingNonBlockingAdmit`). These added methods
are simplified versions of `WaitForEval`, which always wait for all
connected streams to have available tokens before admission.

Resolves: cockroachdb#130186
Release note: None
kvoli added a commit to kvoli/cockroach that referenced this issue Sep 13, 2024
@kvoli
rac2: add kvflowsimulator equivalent simulation test
94487ca 
Add a new simulation test, `TestUsingSimulation`. This test mirrors
`kvflowsimulator.TestUsingSimulation` and its existing datadriven tests
cases.

When comparing the output from each, no behavioral differences show up,
only intentionally different metric names.

Note that like the `kvflowsimulator.TestUsingSimulation`, the simulation
test needs to be deterministic and therefore only uses a single
goroutine, to avoid goroutine scheduling causing non-determinism. As
such, helper methods have been added to facilitate waiting for available
tokens directly on a stream's token counter and on a range in a
non-blocking manner (`testingNonBlockingAdmit`). These added methods
are simplified versions of `WaitForEval`, which always wait for all
connected streams to have available tokens before admission.

Resolves: cockroachdb#130186
Release note: None
kvoli added a commit to kvoli/cockroach that referenced this issue Sep 13, 2024
@kvoli
rac2: add kvflowsimulator equivalent simulation test
879b38d 
Add a new simulation test, `TestUsingSimulation`. This test mirrors
`kvflowsimulator.TestUsingSimulation` and its existing datadriven tests
cases.

When comparing the output from each, no behavioral differences show up,
only intentionally different metric names.

Note that like the `kvflowsimulator.TestUsingSimulation`, the simulation
test needs to be deterministic and therefore only uses a single
goroutine, to avoid goroutine scheduling causing non-determinism. As
such, helper methods have been added to facilitate waiting for available
tokens directly on a stream's token counter and on a range in a
non-blocking manner (`testingNonBlockingAdmit`). These added methods
are simplified versions of `WaitForEval`, which always wait for all
connected streams to have available tokens before admission.

Resolves: cockroachdb#130186
Release note: None
kvoli added a commit to kvoli/cockroach that referenced this issue Sep 13, 2024
@kvoli
rac2: add kvflowsimulator equivalent simulation test
65bc31e 
Add a new simulation test, `TestUsingSimulation`. This test mirrors
`kvflowsimulator.TestUsingSimulation` and its existing datadriven tests
cases.

When comparing the output from each, no behavioral differences show up,
only intentionally different metric names.

Note that like the `kvflowsimulator.TestUsingSimulation`, the simulation
test needs to be deterministic and therefore only uses a single
goroutine, to avoid goroutine scheduling causing non-determinism. As
such, helper methods have been added to facilitate waiting for available
tokens directly on a stream's token counter and on a range in a
non-blocking manner (`testingNonBlockingAdmit`). These added methods
are simplified versions of `WaitForEval`, which always wait for all
connected streams to have available tokens before admission.

Resolves: cockroachdb#130186
Release note: None
craig bot pushed a commit that referenced this issue Sep 16, 2024
@kvoli
Merge #130446
5988cf7 
130446: rac2: add kvflowsimulator equivalent simulation test r=sumeerbhola a=kvoli

Add a new simulation test, `TestUsingSimulation`. This test mirrors
`kvflowsimulator.TestUsingSimulation` and its existing datadriven tests
cases.

When comparing the output from each, no behavioral differences show up,
only intentionally different metric names.

Note that like the `kvflowsimulator.TestUsingSimulation`, the simulation
test needs to be deterministic and therefore only uses a single
goroutine, to avoid goroutine scheduling causing non-determinism. As
such, helper methods have been added to facilitate waiting for available
tokens directly on a stream's token counter and on a range in a
non-blocking manner (`testingNonBlockingAdmit`). These added methods
are simplified versions of `WaitForEval`, which always wait for all
connected streams to have available tokens before admission.

Resolves: #130186
Release note: None

Co-authored-by: Austen McClernon <[email protected]>
@craig craig bot closed this as completed in 15aefd7 Sep 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@kvoli kvoli

Labels
A-replication-admission-control-v2 Related to introduction of replication AC v2 C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) T-kv KV Team
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

rac2: add kvflowsimulator equivalent simulation test kvoli/cockroach
1 participant
@kvoli