Add words about read refreshing to the transaction design page #2684
Conversation
|
This change is |
|
cc @jordanlewis who requested some docs for the refreshing |
|
|
||
| 2. `TxnB` tries to push `TxnA`'s timestamp forward. | ||
|
|
||
| This succeeds only in the case that `TxnA` has snapshot isolation and `TxnB`'s operation is a read. In this case, the [write skew](https://en.wikipedia.org/wiki/Snapshot_isolation) anomaly occurs. | ||
|
|
||
| 3. `TxnB` enters the `PushTxnQueue` to wait for `TxnA` to complete. | ||
|
|
||
| Additionally, the following types of conflicts that don't involve running into intents can arise: | ||
|
|
||
| - **Write after read**, when a write with a lower timestamp encouters a later read. This is handled through the [Timestamp Cache](#timestamp-cache). |
| Additionally, the following types of conflicts that don't involve running into intents can arise: | ||
|
|
||
| - **Write after read**, when a write with a lower timestamp encouters a later read. This is handled through the [Timestamp Cache](#timestamp-cache). | ||
| - **Read within uncertainty window**, when a read encounters a value with a higher timestamp but it's ambiguous whether the value should be considered to be in the future or in the past of the transaction because of possible *clock skew*. This is handled by attempting to push the transaction's timestamp beyond the uncertain value (see [read refreshing](#read-refreshing)). |
There was a problem hiding this comment.
s/attempting to push.../attempting to forward the transaction's timestamp just beyond the uncertain value's timestamp/ (I don't think push is right in this context).
You might also add: "If the transaction timestamp forwarding and refreshing is unsuccessful, then the transaction must be retried at the forwarded timestamp. Note that on successive retries, reads will never encounter uncertainty issues on any node which was previously visited."
|
|
||
| ### Read Refreshing | ||
|
|
||
| Whenever a transaction's timestamp has been pushed, additional checks are required before allowing serializable transactions to commit at the pushed timestamp: we need to check that whatever the transaction has previously read at its original timestamp hasn't changed before the pushed timestamp. This is done by keeping track of all the reads using a dedicted `RefreshRequest`. If this succeeds, the transaction is allowed to commit (technically, transactions perform this check at commit time if they've been pushed by a different transaction or by the timestamp cache, or they perform the check whenever they encounter a `ReadWithinUncertaintyIntervalError` before continuing). |
There was a problem hiding this comment.
I would re-word this to remove the use of first person pronouns ("we"). How about:
"any values which the transaction previously read must be checked to verify that no writes have subsequently occurred between the original transaction timestamp and the forwarded transaction timestamp. This check prevents serializable isolation violations."
s/dedicted/dedicated/
s/technically, transactions/usually transactions/
s/before continuing/immediately, before continuing/
|
@andreimatei thanks for embarking on this effort. |
|
Review status: 0 of 1 files reviewed at latest revision, 3 unresolved discussions, all commit checks successful. v2.0/architecture/transaction-layer.md, line 152 at r1 (raw file): Previously, spencerkimball (Spencer Kimball) wrote…
Done. v2.0/architecture/transaction-layer.md, line 153 at r1 (raw file): Previously, spencerkimball (Spencer Kimball) wrote…
why isn't "push" the right word? That's what we generally use when we talk about a transaction's timestamp being changed. I understand what we discussed the other day - about how, in the case of writes running into the timestamp cache the transaction can continue reading at its old timestamp, but in the case of uncertainty errors, the txn needs to immediately start reading at the new timestamp, but I don't think these considerations make "push" not valid... I've incorporated your addendum, but I've split it between here and the refresh section. v2.0/architecture/transaction-layer.md, line 182 at r1 (raw file): Previously, spencerkimball (Spencer Kimball) wrote…
Done. Comments from Reviewable |
|
Review status: 0 of 1 files reviewed at latest revision, 3 unresolved discussions, all commit checks successful. v2.0/architecture/transaction-layer.md, line 182 at r1 (raw file): Previously, andreimatei (Andrei Matei) wrote…
I thought the reads were tracked at the Comments from Reviewable |
|
Review status: 0 of 1 files reviewed at latest revision, 3 unresolved discussions, all commit checks successful. v2.0/architecture/transaction-layer.md, line 182 at r1 (raw file): Previously, petermattis (Peter Mattis) wrote…
There's a missing word indeed; I wanted to say "keeping track of all the reads and using a dedicated Comments from Reviewable |
No description provided.