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

Use different workload seeds by default #88566

Closed
renatolabs opened this issue Sep 23, 2022 · 2 comments · Fixed by #95326
Closed

Use different workload seeds by default #88566

renatolabs opened this issue Sep 23, 2022 · 2 comments · Fixed by #95326
Labels
A-testing Testing tools and infrastructure C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) T-testeng TestEng Team

Comments

@renatolabs
Copy link
Contributor

renatolabs commented Sep 23, 2022 •
edited by cockroach-jira-scripts
Loading

Currently, our workloads use a static default for pseudo-random number generator seed, typically 1. For example, see bank [1] and tpcc [2] workloads. To increase the chances of finding anomalous behavior, we should be using different random seeds by default.

This has recently been done for the rand workload [3].

In the case of workloads used in automated tests (such as bank and tpcc), this would also mean that the tests should make sure to pass the same seed to init and run.

[1] https://github.com/cockroachdb/cockroach/blob/master/pkg/workload/bank/bank.go#L69
[2] https://github.com/cockroachdb/cockroach/blob/master/pkg/workload/tpcc/tpcc.go#L187
[3] #88362

Jira issue: CRDB-19959
@renatolabs renatolabs added C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) A-testing Testing tools and infrastructure T-testeng TestEng Team labels Sep 23, 2022
@blathers-crl
Copy link

blathers-crl bot commented Sep 23, 2022

cc @cockroachdb/test-eng

@srosenberg
Copy link
Member

Pasting the note from the PR, so we don't forget,

A time-based seed is not as uniform as a crypto-based seed. cli.Main is already using a seed by reading it from /dev/urandom [1]. I think we should do the same in all the workloads.

[1] https://github.com/cockroachdb/cockroach/blob/master/pkg/cli/cli.go#L53

srosenberg added a commit to srosenberg/cockroach that referenced this issue Sep 29, 2022
@srosenberg
codeowners: add test-eng to owners of pkg/workload
1b0e197 
Add test-eng as a code owner/watcher for pkg/workload.

In light of recent and future improvements [1], [2],
TestEng would prefer to be in sync with all changes
to the workload code. Over time, the team plans to build
expertise in this area.

[1] cockroachdb#88362
[2] cockroachdb#88566

Release note: None
Release justification: test only change
@srosenberg srosenberg mentioned this issue Sep 29, 2022
Merged
craig bot pushed a commit that referenced this issue Sep 30, 2022
@erikgrinaker @srosenberg
Merge #89014 #89026 #89030
3e03176 
89014: jobs: Clear out claim info when pausing r=miretskiy a=miretskiy

Clear out job claim information when job is paused. Clearing out claim information is beneficial since it allows operator to pause/resume job if they want to try to move job coordinator to another node.

Addresses #82698

Release note: none

89026: kvserver: add `SmallEngineBlocks` testing knob and metamorphic params r=erikgrinaker a=erikgrinaker

`@cockroachdb/repl-prs` to do the main review, tagging other teams for visibility/review of metamorphic test params.

Resolves #86648.

---

**kvserver: add `SmallEngineBlocks` testing knob**

This patch adds a store testing knob `SmallEngineBlocks` which
configures Pebble with a block size of 1 byte. This will store every key
in a separate block, which can provoke bugs in time-bound iterators.

Release note: None
  
**sql/logictest: add metamorphic test param for small engine blocks**

Uses a Pebble block size of 1 byte, to provoke bugs in time-bound
iterators.

Release note: None
  
**kvserver/rangefeed: add metamorphic test param for small engine blocks**

Uses a Pebble block size of 1 byte, to provoke bugs in time-bound
iterators.

Release note: None
  
**kvserver/gc: add metamorphic test param for small engine blocks**

Uses a Pebble block size of 1 byte, to provoke bugs in time-bound
iterators.

Release note: None

  
**backupccl: add metamorphic test param for small engine blocks**

Uses a Pebble block size of 1 byte, to provoke bugs in time-bound
iterators.

Release note: None

89030: codeowners: add test-eng to owners of pkg/workload r=srosenberg a=srosenberg

Add test-eng as a code owner/watcher for pkg/workload.

In light of recent and future improvements [1], [2], TestEng would prefer to be in sync with all changes to the workload code. Over time, the team plans to build expertise in this area.

[1] #88362 [2] #88566

Release note: None
Release justification: test only change

Co-authored-by: Yevgeniy Miretskiy <[email protected]>
Co-authored-by: Erik Grinaker <[email protected]>
Co-authored-by: Stan Rosenberg <[email protected]>
@renatolabs renatolabs mentioned this issue Jan 16, 2023
Merged
craig bot pushed a commit that referenced this issue Jan 23, 2023
@renatolabs @kvoli
Merge #95326 #95683
c08b21e 
95326: workload: use different random seeds by default r=srosenberg a=renatolabs

This changes most of the workloads that take a `--seed` command line flag so that they use a different, randomly generated seed on each run.  Previously, they would all default to `1`, making every run of the workload produce the same data and operations. That behavior is good for reproducing a specific pattern or behavior, but workloads that have a random element should exploit that randomness by default.

When a workload is invoked, a random seed is generated and logged. Users are still able to specify their own seeds by passing the `--seed` flag.

Resolves #88566.

Release note (cli change): workloads that take a `--seed` argument
used to default to `1`. Now, they use a randomly generated seed in
each run. Users can still pass a custom seed with the `--seed` flag.

95683: kvserver: decrease log verbosity when no rebalance r=knz,andrewbaptist a=kvoli

Previously, the replicate queue would log every time it could not find a rebalance target for a voter or non-voter. This can spam the logs on startup with messages. This commit reduces the logging verbosity of these lines to 2, from default on.

resolves: #95681

Release note: None

Co-authored-by: Renato Costa <[email protected]>
Co-authored-by: Austen McClernon <[email protected]>
@craig craig bot closed this as completed in 7b71595 Jan 23, 2023
renatolabs added a commit to renatolabs/cockroach that referenced this issue Jan 23, 2023
@renatolabs
workload: use different random seeds by default
fabc0ec 
This changes most of the workloads that take a `--seed` command line
flag so that they use a different, randomly generated seed on each
run.  Previously, they would all default to `1`, making every run of
the workload produce the same data and operations. That behavior is
good for reproducing a specific pattern or behavior, but workloads
that have a random element should exploit that randomness by
default.

When a workload is invoked, a random seed is generated and
logged. Users are still able to specify their own seeds by passing the
`--seed` flag.

Resolves cockroachdb#88566.

Release note (cli change): workloads that take a `--seed` argument
used to default to `1`. Now, they use a randomly generated seed in
each run. Users can still pass a custom seed with the `--seed` flag.
@renatolabs renatolabs mentioned this issue Jan 23, 2023
Merged
renatolabs added a commit to renatolabs/cockroach that referenced this issue Feb 28, 2023
@renatolabs
workload: use different random seeds by default
14a3746 
This changes most of the workloads that take a `--seed` command line
flag so that they use a different, randomly generated seed on each
run.  Previously, they would all default to `1`, making every run of
the workload produce the same data and operations. That behavior is
good for reproducing a specific pattern or behavior, but workloads
that have a random element should exploit that randomness by
default.

When a workload is invoked, a random seed is generated and
logged. Users are still able to specify their own seeds by passing the
`--seed` flag.

Resolves cockroachdb#88566.

Release note (cli change): workloads that take a `--seed` argument
used to default to `1`. Now, they use a randomly generated seed in
each run. Users can still pass a custom seed with the `--seed` flag.
@cockroach-teamcity cockroach-teamcity mentioned this issue Mar 1, 2023
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
A-testing Testing tools and infrastructure C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) T-testeng TestEng Team
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

workload: use different random seeds by default renatolabs/cockroach
2 participants
@srosenberg @renatolabs