-
Notifications
You must be signed in to change notification settings - Fork 3.8k
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
roachtest: add cluster settings operations #123806
roachtest: add cluster settings operations #123806
Conversation
3100868
to
1324f13
Compare
1324f13
to
45a1d68
Compare
@@ -29,7 +29,7 @@ func CheckDependencies( | |||
for _, dep := range spec.Dependencies { | |||
switch dep { | |||
case registry.OperationRequiresNodes: | |||
if len(c.Nodes()) == 0 { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's counter-intuitive that c.Nodes()
denotes the empty set (of nodes). We should probably update the API; my quick audit of all the callers didn't yield any other than the above.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep, took me a few to realise why OperationRequiresNodes
wasn't working.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for catching this!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
} | ||
} | ||
|
||
// timeBasedValues returns a function that returns a value from the given list |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nice!
{ | ||
Name: "kv.expiration_leases_only.enabled", | ||
Generator: timeBasedValues(timeSupplier, []string{"true", "false"}, 24*7*time.Minute), | ||
Owner: registry.OwnerTestEng, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe the owner for this should be KV?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, that makes sense. I'll update the owner.
return fmt.Sprintf("%d", rng.Intn(246)+5) | ||
}), | ||
Owner: registry.OwnerTestEng, | ||
}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe we should add one for kv.snapshot_receiver.excise.enabled
as well, but I'm good with this as a follow-up change.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good idea, merging this one for now. Will create a follow-up PR with some additional cluster settings, I have some others in mind as well.
@@ -29,7 +29,7 @@ func CheckDependencies( | |||
for _, dep := range spec.Dependencies { | |||
switch dep { | |||
case registry.OperationRequiresNodes: | |||
if len(c.Nodes()) == 0 { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for catching this!
This change adds operations to mutate cluster settings. The values are mutated based on a preset frequency and RNG. For example, if the frequency of a cluster setting operation is set to "30 minutes" it will only change to a new value every 30 minutes, even if the operation has been run multiple times within the same 30-minute window. Running in the same window will just set the same value again. Epic: None Release Note: None
Epic: None Release Note: None
Epic: None Release Notes: None
45a1d68
to
1beef0c
Compare
TFTR! bors r=itsbilal |
This change adds operations to mutate cluster settings. The values are mutated
based on a preset frequency and RNG. For example, if the frequency of a cluster
setting operation is set to "30 minutes" it will only change to a new value
every 30 minutes, even if the operation has been run multiple times within the
same 30-minute window. Running in the same window will just set the same value
again.
Epic: None
Release Note: None