storage: break some dependencies on inconsistent reads with intent interleaving #109794
Conversation
…terleaving Informs cockroachdb#109647. This commit eliminates a pair of dependencies in mvccPutInternal on inconsistent reads and intent interleaving when a transactional write wanted to read below its own intent. Instead, we can simply read at the timestamp preceding the intent, which is what the pebbleMVCCScanner was doing under the hood anyway. 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. |
|
This change is |
There was a problem hiding this comment.
Reviewed 1 of 1 files at r1, all commit messages.
Reviewable status:complete! 1 of 0 LGTMs obtained (waiting on @nvanbenschoten)
pkg/storage/mvcc.go line 1777 at r1 (raw file):
// value on the key, we read below our previous intents here. metaTimestamp := meta.Timestamp.ToTimestamp() exVal, _, err = mvccGet(ctx, iter, key, metaTimestamp.Prev(), MVCCGetOptions{
this is still an intentInterleavingIter, yes?
There was a problem hiding this comment.
TFTR!
bors r=sumeerbhola
Reviewable status:
pkg/storage/mvcc.go line 1777 at r1 (raw file):
Previously, sumeerbhola wrote…
this is still an intentInterleavingIter, yes?
It is for now, yes, but it no longer needs to be. I have a branch that strips out the intentInterleavingIter from mutation paths and this change was needed to get unit tests passing. That PR will be published some time in the next week.
|
Build succeeded: |
Informs #109647.
This commit eliminates a pair of dependencies in mvccPutInternal on inconsistent reads and intent interleaving when a transactional write wanted to read below its own intent. Instead, we can simply read at the timestamp preceding the intent, which is what the pebbleMVCCScanner was doing under the hood anyway.
Release note: None