kv: support locking Get requests#59023
Conversation
e6c4349 to
5d97dd6
Compare
This commit adds support for Get requests to acquire unreplicated locks in cases where they find a present key-value. This is supported by a new field on GetRequest called KeyLocking, which mirrors the field of the same name found on ScanRequest and ReverseScanRequest. Much of this commit was modeled after a98be1f, which was the original commit that added locking support to ScanRequest and ReverseScanRequest. The intention is to use this functionality to fix cockroachdb#46639.
This commit extends kvnemesis to issue locking GetRequests. Much of this is modeled after a98be1f, which added support to kvnemesis for locking ScanRequests.
5d97dd6 to
c955e88
Compare
aayushshah15
left a comment
There was a problem hiding this comment.
The pointer to your SELECT FOR UPDATE PR was very helpful.
but most of my review on it was me understanding it rather than scrutinizing it. I assume that that's okay since it's only reusing the machinery introduced in that above PR.
Reviewable status:
complete! 1 of 0 LGTMs obtained (waiting on @nvanbenschoten)
pkg/kv/db_test.go, line 90 at r1 (raw file):
defer s.Stopper().Stop(context.Background()) result, err := db.GetForUpdate(context.Background(), "aa")
Do you know why these two Get tests are written the way they are? It seems like they're intentionally "simplistic".
pkg/roachpb/api.go, line 1170 at r1 (raw file):
func (gr *GetRequest) flags() int { maybeLocking := flagForLockStrength(gr.KeyLocking)
For my understanding. why the maybe here? Is it because the unreplicated lock is only acquired if the key exists?
nvb
left a comment
There was a problem hiding this comment.
TFTR!
bors r+
Reviewable status:
complete! 1 of 0 LGTMs obtained (waiting on @aayushshah15)
pkg/kv/db_test.go, line 90 at r1 (raw file):
Previously, aayushshah15 (Aayush Shah) wrote…
Do you know why these two
Gettests are written the way they are? It seems like they're intentionally "simplistic".
They seem to be quite old tests. In fact, they were originally Example-style tests, and were converted to normal unit tests in 12f8d1d in response to #6101.
pkg/roachpb/api.go, line 1170 at r1 (raw file):
Previously, aayushshah15 (Aayush Shah) wrote…
For my understanding. why the
maybehere? Is it because the unreplicated lock is only acquired if the key exists?
I think it's actually because if KeyLocking is set to None then this flag is not set. So maybe is indicating the flag may or may or may not be non-zero.
|
Build succeeded: |
This commit adds support for
Getrequests to acquire unreplicated locks in cases where they find a present key-value. This is supported by a new field onGetRequestcalledKeyLocking, which mirrors the field of the same name found onScanRequestandReverseScanRequest.Much of this commit was modeled after a98be1f, which was the original commit that added locking support to ScanRequest and ReverseScanRequest.
The intention is to use this functionality to fix #46639.