*: replace some usages of global rand#155797
Merged
craig[bot] merged 2 commits intocockroachdb:masterfrom Oct 22, 2025
Merged
Conversation
Release note: None
Member
a776c5f to
cd4bef7
Compare
When we use `rand.*` methods like `math/rand.Intn`, under the hood we hit the global rand source, which is protected by a mutex, so we could be subject to contention on that lock. This commit updates some of such usages in favor of object-specific rand source to avoid that. Note that allocating a separate rand source is not free (the allocation itself is about 5KiB in size), so we modify spots where the lifecycle matches that of the server or when the affected code is heavy already / not on the hot path (also when it's clear that the access is from within a single goroutine). Release note: None
cd4bef7 to
8a8e228
Compare
|
It looks like your PR touches production code but doesn't add or edit any test code. Did you consider adding tests to your PR? 🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf. |
michae2
approved these changes
Oct 22, 2025
Collaborator
michae2
left a comment
There was a problem hiding this comment.
@michae2 reviewed 1 of 1 files at r1, 11 of 11 files at r2, all commit messages.
Reviewable status:complete! 1 of 0 LGTMs obtained (waiting on @yuzefovich)
Member
Author
|
TFTR! bors r+ |
craig bot
pushed a commit
that referenced
this pull request
Oct 22, 2025
155616: rangefeed: add assertion that OnError is called on disconnected registration r=stevendanna a=stevendanna Epic: none Release note: None 155797: *: replace some usages of global rand r=yuzefovich a=yuzefovich **sem/builtins: use RNG from eval context for 'st_generatepoints'** ***: replace some usages of global rand** When we use `rand.*` methods like `math/rand.Intn`, under the hood we hit the global rand source, which is protected by a mutex, so we could be subject to contention on that lock. This commit updates some of such usages in favor of object-specific rand source to avoid that. Note that allocating a separate rand source is not free (the allocation itself is about 5KiB in size), so we modify spots where the lifecycle matches that of the server or when the affected code is heavy already / not on the hot path (also when it's clear that the access is from within a single goroutine). Epic: None Release note: None 155861: kvserver: deflake TestPromoteNonVoterInAddVoter r=arulajmani a=arulajmani This test could previously fail because a lease transfer could race with a split. Without the split, the effects of the span config (that the test asserts on) will not apply. We sidestep this problem by removing the need of a split entirely -- by tuning some cluster settings to ensure that every table is on its own range to begin with. H/t to `@pav-kv` for the analysis on this one. Closes #155828 Closes #155747 Closes #155316 Closes #154773 Closes #134383 Release note: None Co-authored-by: Steven Danna <danna@cockroachlabs.com> Co-authored-by: Yahor Yuzefovich <yahor@cockroachlabs.com> Co-authored-by: Arul Ajmani <arulajmani@gmail.com>
Contributor
|
Build failed (retrying...): |
Contributor
rishabh7m
added a commit
to crltest-issue-syncing/cockroach-fork
that referenced
this pull request
Nov 20, 2025
rishabh7m
added a commit
to crltest-issue-syncing/cockroach-fork
that referenced
this pull request
Nov 21, 2025
rishabh7m
added a commit
to crltest-issue-syncing/cockroach-fork
that referenced
this pull request
Nov 21, 2025
rishabh7m
added a commit
to crltest-issue-syncing/cockroach-fork
that referenced
this pull request
Dec 1, 2025
rishabh7m
added a commit
to crltest-issue-syncing/cockroach-fork
that referenced
this pull request
Dec 10, 2025
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
sem/builtins: use RNG from eval context for 'st_generatepoints'
*: replace some usages of global rand
When we use
rand.*methods likemath/rand.Intn, under the hood we hitthe global rand source, which is protected by a mutex, so we could be subject
to contention on that lock. This commit updates some of such usages in
favor of object-specific rand source to avoid that. Note that allocating
a separate rand source is not free (the allocation itself is about 5KiB
in size), so we modify spots where the lifecycle matches that of the
server or when the affected code is heavy already / not on the hot path
(also when it's clear that the access is from within a single
goroutine).
Epic: None
Release note: None