-
Notifications
You must be signed in to change notification settings - Fork 3.7k
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: s/queuedWriters/queuedLockingRequests/g #109454
concurrency: s/queuedWriters/queuedLockingRequests/g #109454
Conversation
The new name is more apt for what is being stored in this list. No functional change, but I did clean up some commentary here. Epic: none Release note: None
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 41 of 41 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 1510 at r1 (raw file):
// // Non-locking readers are held in a separate list to the list of // waitingReaders, and they make no claims on unheld locks like locking
Is this supposed to say "list of waiting locking requests"? The sentence could be made more clear.
pkg/kv/kvserver/concurrency/lock_table.go
line 1577 at r1 (raw file):
// This is a deadlock caused by the lock table unless req2 partially // breaks the claim at A. queuedLockingRequests list.List
This could have also been queuedLockers
or queuedLockingReqs
if you were looking for something a little more compact, but this works too!
We renamed the receiver when lockState was changed to keyLocks, but we never renamed all references to l.mu. Epic: none Release note: None
fe8734e
to
3c6df30
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!
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 1510 at r1 (raw file):
Previously, nvanbenschoten (Nathan VanBenschoten) wrote…
Is this supposed to say "list of waiting locking requests"? The sentence could be made more clear.
Yeah. I simplified this paragraph a bit.
pkg/kv/kvserver/concurrency/lock_table.go
line 1577 at r1 (raw file):
Previously, nvanbenschoten (Nathan VanBenschoten) wrote…
This could have also been
queuedLockers
orqueuedLockingReqs
if you were looking for something a little more compact, but this works too!
Mild preference for queuedLockingRequests
because it rolls off the tongue better when used in comments, so I'll keep it.
Build succeeded: |
See individual commits for details.