Skip to content

kv: establish new read snapshot on statement/batch boundaries under RC#104362

Merged
craig[bot] merged 1 commit intocockroachdb:masterfrom
nvb:nvanbenschoten/stepReadCommitted
Jun 14, 2023
Merged

kv: establish new read snapshot on statement/batch boundaries under RC#104362
craig[bot] merged 1 commit intocockroachdb:masterfrom
nvb:nvanbenschoten/stepReadCommitted

Conversation

@nvb
Copy link
Copy Markdown
Contributor

@nvb nvb commented Jun 5, 2023

Closes #100133.

This commit teaches Read Committed transactions to establish a new external "read snapshot" on each SQL statement or KV batch boundary.

The per-statement/batch read snapshot logic is integrated into the transaction Stepping infrastructure in an analogous manner to how the transaction's internal readSeq is advanced. For transactions with stepping enabled (e.g. SQL transactions), the Step API now advances the transaction's external read snapshot (i.e. ReadTimestamp) to a timestamp captured from the local HLC clock. This ensures that subsequent read-only operations observe the writes of other transactions that were committed before the time the statement began. For transactions with stepping disabled (e.g. raw KV transactions), each batch implicitly advances the read snapshot. This ensures that the batch observes the writes of other transactions that were committed before the batch was issued.

This read snapshot will be at least as recent as the previous read snapshot, and will typically be more recent (i.e. it will never regress). Consistency with prior reads in the transaction is not maintained, so reads of previously read keys may not be "repeatable" and may observe "phantoms". On the other hand, by not maintaining consistency between read snapshots, isolation-related retries (write-write conflicts) and consistency-related retries (uncertainty errors) have a higher chance of being refreshed away (client-side or server-side) without the need for client intervention (i.e. without requiring a statement-level retry). This benefit can be seen in TestTxnCoordSenderRetries.

As described in the Read Committed RFC, the transaction's uncertainty interval is not reset when establishing a new read snapshot. See section "Read Uncertainty Intervals" of the RFC for the rationale behind this decision.

Release note: None

@nvb nvb requested a review from arulajmani June 5, 2023 20:31
@nvb nvb requested a review from a team as a code owner June 5, 2023 20:31
@cockroach-teamcity
Copy link
Copy Markdown
Member

This change is Reviewable

@nvb nvb force-pushed the nvanbenschoten/stepReadCommitted branch 2 times, most recently from 13deade to c5aed0d Compare June 8, 2023 03:25
Copy link
Copy Markdown
Collaborator

@arulajmani arulajmani left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:lgtm:

Reviewed 4 of 4 files at r1, 4 of 5 files at r2, 4 of 4 files at r3, all commit messages.
Reviewable status: :shipit: complete! 1 of 0 LGTMs obtained (waiting on @nvanbenschoten)


pkg/kv/kvclient/kvcoord/txn_test.go line 390 at r3 (raw file):

	defer log.Scope(t).Close(t)

	run := func(isoLevel isolation.Level, mode kv.SteppingMode, step bool, exp bool) {

nit: consider renaming exp to something more descriptive.

Closes cockroachdb#100133.

This commit teaches Read Committed transactions to establish a new
external "read snapshot" on each SQL statement or KV batch boundary.

The per-statement/batch read snapshot logic is integrated into the
transaction `Stepping` infrastructure in an analogous manner to how the
transaction's internal `readSeq` is advanced. For transactions with
stepping enabled (e.g. SQL transactions), the `Step` API now advances
the transaction's external read snapshot (i.e. `ReadTimestamp`) to a
timestamp captured from the local HLC clock. This ensures that
subsequent read-only operations observe the writes of other transactions
that were committed before the time the statement began. For transactions
with stepping disabled (e.g. raw KV transactions), each batch implicitly
advances the read snapshot. This ensures that the batch observes the
writes of other transactions that were committed before the batch was
issued.

This read snapshot will be at least as recent as the previous read
snapshot, and will typically be more recent (i.e. it will never
regress). Consistency with prior reads in the transaction is not
maintained, so reads of previously read keys may not be "repeatable" and
may observe "phantoms". On the other hand, by not maintaining
consistency between read snapshots, isolation-related retries
(write-write conflicts) and consistency-related retries (uncertainty
errors) have a higher chance of being refreshed away (client-side or
server-side) without need for client intervention (i.e. without
requiring a statement-level retry). This benefit can be seen in
`TestTxnCoordSenderRetries`.

As described in the Read Committed RFC, the transaction's uncertainty
interval is not reset when establishing a new read snapshot. See section
"Read Uncertainty Intervals" of the RFC for the rationale behind this
decision.

Release note: None
Copy link
Copy Markdown
Contributor Author

@nvb nvb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TFTR!

bors r+

Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained (and 1 stale) (waiting on @arulajmani)


pkg/kv/kvclient/kvcoord/txn_test.go line 390 at r3 (raw file):

Previously, arulajmani (Arul Ajmani) wrote…

nit: consider renaming exp to something more descriptive.

Done.

@craig
Copy link
Copy Markdown
Contributor

craig bot commented Jun 14, 2023

Build succeeded:

@craig craig bot merged commit 13addf5 into cockroachdb:master Jun 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

kv: establish new read snapshot on statement boundaries under RC

3 participants