rac2: reduce tokenCounter mutex contention #143518
Conversation
![blathers[crl]](https://avatars.githubusercontent.com/in/59700?s=40&u=95b789b1d594ac6b74f377aa7555d8d98d63240d&v=4){kind=avatar}
|
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. |
|
This change is |
There was a problem hiding this comment.
see https://cockroachlabs.slack.com/archives/C06UFBJ743F/p1743018300491679 for motivation
Reviewable status:
complete! 0 of 0 LGTMs obtained (waiting on @kvoli)
There was a problem hiding this comment.
Reviewed 1 of 1 files at r1, all commit messages.
Reviewable status:
pkg/kv/kvserver/kvflowcontrol/rac2/token_counter.go line 326 at r1 (raw file):
tokensAvailable := t.tokensLocked(wc) if tokensAvailable <= 0 { t.mu.Unlock()
This will need a // nolint:deferunlockcheck inline comment inline, or at the top of the function iirc. Ditto for adjustLockedAndUnlock.
There was a problem hiding this comment.
Reviewable status:
pkg/kv/kvserver/kvflowcontrol/rac2/token_counter.go line 326 at r1 (raw file):
Previously, kvoli (Austen) wrote…
This will need a
// nolint:deferunlockcheckinline comment inline, or at the top of the function iirc. Ditto foradjustLockedAndUnlock.
I couldn't figure out the right dev lint ... incantation to trigger this in my local build -- do you happen to know?
|
never mind: |
The metrics update and logging are moved out of the critical section. Epic: none Release note: None
sumeerbhola
force-pushed
the
token_counter_contention
branch
from
44d44ad to
80484e0
Compare
|
TFTR! |
There was a problem hiding this comment.
Reviewable status:
pkg/kv/kvserver/kvflowcontrol/rac2/token_counter.go line 326 at r1 (raw file):
Previously, sumeerbhola wrote…
I couldn't figure out the right
dev lint ...incantation to trigger this in my local build -- do you happen to know?
I've never gotten this to work, I usually just push the changes and see what happens with engflow.
|
bors r=kvoli |
The metrics update and logging are moved out of the critical section.
Epic: none
Release note: None