Skip to content
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.

Sign up for GitHub

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

Jump to bottom

kv: establish new read snapshot on statement boundaries under RC #100133

Closed
nvanbenschoten opened this issue Mar 30, 2023 · 0 comments · Fixed by #104362
Closed

kv: establish new read snapshot on statement boundaries under RC #100133

nvanbenschoten opened this issue Mar 30, 2023 · 0 comments · Fixed by #104362
Assignees
@nvanbenschoten
Labels
A-kv-client Relating to the KV client and the KV interface. A-kv-transactions Relating to MVCC and the transactional model. A-read-committed Related to the introduction of Read Committed C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) T-kv KV Team

Comments

@nvanbenschoten
Copy link
Member

nvanbenschoten commented Mar 30, 2023 •
edited by cockroach-jira-scripts
Loading

With the introduction of the read committed isolation level, transactions running under the isolation level will establish a new read snapshot on each statement boundary. This should live in Txn.Step/TxnCoordSender.Step.

cockroach/pkg/kv/kvserver/concurrency/isolation/levels.proto

Lines 93 to 95 in f9ac216

// Read Snapshot Scope: Does the isolation level allow transactions to operate
// across multiple read snapshots? If not, a single read snapshot is used for
// the entire transaction. If so, what is the scope of each read snapshot?

Jira issue: CRDB-26578
@nvanbenschoten nvanbenschoten added C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) A-kv-client Relating to the KV client and the KV interface. A-kv-transactions Relating to MVCC and the transactional model. T-kv KV Team A-read-committed Related to the introduction of Read Committed labels Mar 30, 2023
This was referenced Jun 1, 2023
Closed
Closed
@nvanbenschoten nvanbenschoten mentioned this issue Jun 5, 2023
Merged
nvanbenschoten added a commit to nvanbenschoten/cockroach that referenced this issue Jun 5, 2023
@nvanbenschoten
kv: establish new read snapshot on statement/batch boundaries under RC
13deade 
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
nvanbenschoten added a commit to nvanbenschoten/cockroach that referenced this issue Jun 8, 2023
@nvanbenschoten
kv: establish new read snapshot on statement/batch boundaries under RC
c5aed0d 
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
@nvanbenschoten nvanbenschoten mentioned this issue Jun 14, 2023
Closed
craig bot pushed a commit that referenced this issue Jun 14, 2023
@nvanbenschoten
Merge #104362
13addf5 
104362: kv: establish new read snapshot on statement/batch boundaries under RC r=nvanbenschoten a=nvanbenschoten

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

Co-authored-by: Nathan VanBenschoten <[email protected]>
@craig craig bot closed this as completed in f8e5a8a Jun 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@nvanbenschoten nvanbenschoten

Labels
A-kv-client Relating to the KV client and the KV interface. A-kv-transactions Relating to MVCC and the transactional model. A-read-committed Related to the introduction of Read Committed C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) T-kv KV Team
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

kv: establish new read snapshot on statement/batch boundaries under RC nvanbenschoten/cockroach
1 participant
@nvanbenschoten