storage/roachpb: various cleanup around Refresh{Range}Request #34566
Conversation
Release note: None
We no longer need special cases for these two request types in updateTimestampCache. Release note: None
|
This change is |
There was a problem hiding this comment.
Reviewed 2 of 2 files at r1, 2 of 2 files at r2, 2 of 2 files at r3.
Reviewable status:complete! 0 of 0 LGTMs obtained (waiting on @nvanbenschoten)
pkg/storage/batcheval/cmd_refresh.go, line 59 at r3 (raw file):
// !ts.Less(h.Txn.PrevRefreshTimestamp) // This could avoid failed refreshes due to requests performed after // earlier refreshes.
How would that happen? The previous refresh seals the timestamp cache for PrevRefreshTimestamp. Doesn't that guarantee that nothing new ever happens below?
pkg/storage/batcheval/cmd_refresh_range.go, line 55 at r3 (raw file):
Tombstones: true, }, func(kv roachpb.KeyValue) (bool, error) { // TODO(nvanbenschoten): This is pessimistic. We only need to check
Ditto.
Also improve a comment. Release note: None
There was a problem hiding this comment.
Reviewable status:
pkg/storage/batcheval/cmd_refresh.go, line 59 at r3 (raw file):
Previously, tbg (Tobias Grieger) wrote…
How would that happen? The previous refresh seals the timestamp cache for PrevRefreshTimestamp. Doesn't that guarantee that nothing new ever happens below?
This is talking about requests issued for the first time after a refresh has already happened. For instance, consider:
txnA (orig=1) - read X
txnA (orig=1) - read Y, find intent at ts 3
txnB (orig=2) - write Z @ ts=2
txnA refresh to 3
txnA (ref=3) - read Z and respect the write to Z
txnA refresh to 4 for some other reason
When refreshing key Z, we shouldn't fail because of the intent at time 2.
I tried to make the comment more clear.
nvanbenschoten
force-pushed
the
nvanbenschoten/replicaCleanup
branch
from
29149ac
to
e7d470a
Compare
There was a problem hiding this comment.
Reviewed 2 of 2 files at r4.
Reviewable status:
|
bors r+ |
Build succeeded |
This PR includes a few small changes that I noticed while investigating #34025.