-
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
concurrency: use generic lists in the lock table #109511
Conversation
Now that cda4fa2 has landed, we can make use of generic lists in a few places in the lock table. Epic: none Release note: None
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. |
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.
Reviewed 2 of 2 files at r1, 1 of 1 files at r2, all commit messages.
Reviewable status: complete! 1 of 0 LGTMs obtained (waiting on @arulajmani)
pkg/kv/kvserver/concurrency/lock_table.go
line 1476 at r1 (raw file):
// waiting for. // queuedLockingRequests is of requests queued at a key. They may be waiting
"is a list of locking requests"?
pkg/kv/kvserver/concurrency/lock_table.go
line 1576 at r1 (raw file):
queuedLockingRequests list.List[*queuedGuard] // waitingReaders is the list of non-locking reads that are actively waiting.
nit: "is a list"?
Burns down a TODO. Epic: none Release note: None
2b1a106
to
a4dcc18
Compare
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.
TFTR! Thanks for pushing on the generics patch -- this stuff looks way nicer now!
bors r=nvanbenschoten
Reviewable status: complete! 0 of 0 LGTMs obtained (and 1 stale) (waiting on @nvanbenschoten)
pkg/kv/kvserver/concurrency/lock_table.go
line 1476 at r1 (raw file):
Previously, nvanbenschoten (Nathan VanBenschoten) wrote…
"is a list of locking requests"?
🤦
Build succeeded: |
Now that cda4fa2 has landed, we can make use of generic lists in a few places in the lock table.
Epic: none
Release note: None