rfcs: new RFC on fixing the halloween problem #42864
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
This change is |
knz
force-pushed
the
20191129-rfc-halloween
branch
from
704a797
to
90997ed
Compare
knz
force-pushed
the
20191129-rfc-halloween
branch
from
90997ed
to
59b3741
Compare
This was referenced Nov 29, 2019
Merged
|
Well written, LGTM. @jordanlewis make sure you take a look, we'll need to figure out a clean way to integrate this with the new FK stuff. |
Release note: none
knz
force-pushed
the
20191129-rfc-halloween
branch
from
59b3741
to
2f4446b
Compare
|
TFYR! bors r+ |
craig bot
pushed a commit
that referenced
this pull request
Dec 3, 2019
42668: sql: add precision support for TIME/TIMETZ r=otan a=otan Refs: #32565 Refs: #26097 Adding support for precision for both TIME and TIMETZ. This also includes threading through some extra parsing syntax for TIMETZ. ALTER TABLE between TIME and TIMETZ not supported as they have different representations. Release note (sql change): This PR adds new support for precision for TIME types (e.g. TIME(3) will truncate to milliseconds). Previously this would raise syntax errors. 42864: rfcs: new RFC on fixing the halloween problem r=knz a=knz I am splitting this RFC away from the [RFC on savepoints](#41569) because I know recognize it is a different problem with an orthogonal solution. https://github.com/knz/cockroach/blob/20191129-rfc-halloween/docs/RFCS/20191014_halloween.md There is no new idea in this text (I copy-pasted the text from the other RFC) and the implementation PRs are already out, so if the reviewers are satisfied I would gladly already merge the RFC as in-progress or completed. Co-authored-by: Oliver Tan <otan@cockroachlabs.com> Co-authored-by: Raphael 'kena' Poss <knz@thaumogen.net>
Build succeeded |
|
LGTM. Short and sweet. |
knz
added a commit
to knz/cockroach
that referenced
this pull request
Jan 6, 2020
Previously, all individual KV reads performed by a SQL statement were able to observe the most recent KV writes that it performed itself. This is in violation of PostgreSQL's dialect semantics, which mandate that statements can only observe data as per a read snapshot taken at the instant a statement begins execution. Moreover, this invalid behavior causes a real observable bug: a statement that reads and writes to the same table may never complete, as the read part may become able to consume the rows that it itself writes. Or worse, it could cause logical operations to be performed multiple times: https://en.wikipedia.org/wiki/Halloween_Problem This patch fixes it (partially) by exploiting the new KV `Step()` API which decouples the read and write sequence numbers. The fix is not complete however; additional sequence points must also be introduced prior to FK existence checks and cascading actions. See [cockroachdb#42864](cockroachdb#42864) and [cockroachdb#33475](cockroachdb#33475) for details. For now, this patch excludes any mutation that 1) involves a foreign key and 2) does not uyse the new CBO-driven FK logic, from the new (fixed) semantics. When a mutation involves a FK without CBO involvement, the previous (broken) semantics still apply. Release note (bug fix): SQL mutation statements that target tables with no foreign key relationships now correctly read data as per the state of the database when the statement started execution. This is required for compatibility with PostgreSQL and to ensure deterministic behavior when certain operations are parallelized. Prior to this fix, a statement [could incorrectly operate multiple times](https://en.wikipedia.org/wiki/Halloween_Problem) on data that itself was writing, and potentially never terminate. This fix is limited to tables without FK relationships, and for certain operations on tables with FK relationships; in other cases, the fix is not active and the bug is still present. A full fix will be provided in a later release.
craig bot
pushed a commit
that referenced
this pull request
Jan 6, 2020
42854: roachpb,kv: new TxnCoordSender API for step-wise execution r=knz a=knz Supports #42864. This patch introduces the following methods on `client.TxnSender`: ```go // Step creates a sequencing point in the current transaction. A // sequencing point establishes a snapshot baseline for subsequent // read operations: until the next sequencing point, read operations // observe the data at the time the snapshot was established and // ignore writes performed since. // // Before the first step is taken, the transaction operates as if // there was a step after every write: each read to a key is able to // see the latest write before it. This makes the step behavior // opt-in and backward-compatible with existing code which does not // need it. // The method is idempotent. Step() error // DisableStepping disables the sequencing point behavior and // ensures that every read can read the latest write. The // effect remains disabled until the next call to Step(). // The method is idempotent. DisableStepping() error ``` Additionally it implements it in the `TxnCoordSender`. `Step()` is the most important and will be used to introduce sequence points between SQL statements. `DisableStepping()` is a convenience function, so as to avoid introducing many `Step()` calls in the code that performs schema changes. Release note: none Co-authored-by: Raphael 'kena' Poss <knz@thaumogen.net>
knz
added a commit
to knz/cockroach
that referenced
this pull request
Jan 15, 2020
Previously, all individual KV reads performed by a SQL statement were able to observe the most recent KV writes that it performed itself. This is in violation of PostgreSQL's dialect semantics, which mandate that statements can only observe data as per a read snapshot taken at the instant a statement begins execution. Moreover, this invalid behavior causes a real observable bug: a statement that reads and writes to the same table may never complete, as the read part may become able to consume the rows that it itself writes. Or worse, it could cause logical operations to be performed multiple times: https://en.wikipedia.org/wiki/Halloween_Problem This patch fixes it (partially) by exploiting the new KV `Step()` API which decouples the read and write sequence numbers. The fix is not complete however; additional sequence points must also be introduced prior to FK existence checks and cascading actions. See [cockroachdb#42864](cockroachdb#42864) and [cockroachdb#33475](cockroachdb#33475) for details. For now, this patch excludes any mutation that 1) involves a foreign key and 2) does not uyse the new CBO-driven FK logic, from the new (fixed) semantics. When a mutation involves a FK without CBO involvement, the previous (broken) semantics still apply. Release note (bug fix): SQL mutation statements that target tables with no foreign key relationships now correctly read data as per the state of the database when the statement started execution. This is required for compatibility with PostgreSQL and to ensure deterministic behavior when certain operations are parallelized. Prior to this fix, a statement [could incorrectly operate multiple times](https://en.wikipedia.org/wiki/Halloween_Problem) on data that itself was writing, and potentially never terminate. This fix is limited to tables without FK relationships, and for certain operations on tables with FK relationships; in other cases, the fix is not active and the bug is still present. A full fix will be provided in a later release.
knz
added a commit
to knz/cockroach
that referenced
this pull request
Jan 20, 2020
Previously, all individual KV reads performed by a SQL statement were able to observe the most recent KV writes that it performed itself. This is in violation of PostgreSQL's dialect semantics, which mandate that statements can only observe data as per a read snapshot taken at the instant a statement begins execution. Moreover, this invalid behavior causes a real observable bug: a statement that reads and writes to the same table may never complete, as the read part may become able to consume the rows that it itself writes. Or worse, it could cause logical operations to be performed multiple times: https://en.wikipedia.org/wiki/Halloween_Problem This patch fixes it (partially) by exploiting the new KV `Step()` API which decouples the read and write sequence numbers. The fix is not complete however; additional sequence points must also be introduced prior to FK existence checks and cascading actions. See [cockroachdb#42864](cockroachdb#42864) and [cockroachdb#33475](cockroachdb#33475) for details. For now, this patch excludes any mutation that 1) involves a foreign key and 2) does not uyse the new CBO-driven FK logic, from the new (fixed) semantics. When a mutation involves a FK without CBO involvement, the previous (broken) semantics still apply. Release note (bug fix): SQL mutation statements that target tables with no foreign key relationships now correctly read data as per the state of the database when the statement started execution. This is required for compatibility with PostgreSQL and to ensure deterministic behavior when certain operations are parallelized. Prior to this fix, a statement [could incorrectly operate multiple times](https://en.wikipedia.org/wiki/Halloween_Problem) on data that itself was writing, and potentially never terminate. This fix is limited to tables without FK relationships, and for certain operations on tables with FK relationships; in other cases, the fix is not active and the bug is still present. A full fix will be provided in a later release.
knz
added a commit
to knz/cockroach
that referenced
this pull request
Jan 20, 2020
Previously, all individual KV reads performed by a SQL statement were able to observe the most recent KV writes that it performed itself. This is in violation of PostgreSQL's dialect semantics, which mandate that statements can only observe data as per a read snapshot taken at the instant a statement begins execution. Moreover, this invalid behavior causes a real observable bug: a statement that reads and writes to the same table may never complete, as the read part may become able to consume the rows that it itself writes. Or worse, it could cause logical operations to be performed multiple times: https://en.wikipedia.org/wiki/Halloween_Problem This patch fixes it (partially) by exploiting the new KV `Step()` API which decouples the read and write sequence numbers. The fix is not complete however; additional sequence points must also be introduced prior to FK existence checks and cascading actions. See [cockroachdb#42864](cockroachdb#42864) and [cockroachdb#33475](cockroachdb#33475) for details. For now, this patch excludes any mutation that 1) involves a foreign key and 2) does not uyse the new CBO-driven FK logic, from the new (fixed) semantics. When a mutation involves a FK without CBO involvement, the previous (broken) semantics still apply. Release note (bug fix): SQL mutation statements that target tables with no foreign key relationships now correctly read data as per the state of the database when the statement started execution. This is required for compatibility with PostgreSQL and to ensure deterministic behavior when certain operations are parallelized. Prior to this fix, a statement [could incorrectly operate multiple times](https://en.wikipedia.org/wiki/Halloween_Problem) on data that itself was writing, and potentially never terminate. This fix is limited to tables without FK relationships, and for certain operations on tables with FK relationships; in other cases, the fix is not active and the bug is still present. A full fix will be provided in a later release.
craig bot
pushed a commit
that referenced
this pull request
Jan 20, 2020
42862: sql: make SQL statements operate on a read snapshot r=andreimatei a=knz Fixes #33473. Fixes #28842. Informs #41569 and #42864. Previously, all individual KV reads performed by a SQL statement were able to observe the most recent KV writes that it performed itself. This is in violation of PostgreSQL's dialect semantics, which mandate that statements can only observe data as per a read snapshot taken at the instant a statement begins execution. Moreover, this invalid behavior causes a real observable bug: a statement that reads and writes to the same table may never complete, as the read part may become able to consume the rows that it itself writes. Or worse, it could cause logical operations to be performed multiple times: https://en.wikipedia.org/wiki/Halloween_Problem This patch fixes it by exploiting the new KV `Step()` API which decouples the read and write sequence numbers. The fix is not complete however; additional sequence points must also be introduced prior to FK existence checks and cascading actions. See #42864 and #33475 for details. For now, this patch excludes any mutation that involves a foreign key from the new (fixed) semantics. When a mutation involves a FK, the previous (broken) semantics still apply. Release note (bug fix): SQL mutation statements that target tables with no foreign key relationships now correctly read data as per the state of the database when the statement started execution. This is required for compatibility with PostgreSQL and to ensure deterministic behavior when certain operations are parallelized. Prior to this fix, a statement [could incorrectly operate multiple times](https://en.wikipedia.org/wiki/Halloween_Problem) on data that itself was writing, and potentially never terminate. This fix is limited to tables without FK relationships, and for certain operations on tables with FK relationships; in other cases, the fix is not active and the bug is still present. A full fix will be provided in a later release. Co-authored-by: Raphael 'kena' Poss <knz@thaumogen.net>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I am splitting this RFC away from the RFC on savepoints because I know recognize it is a different problem with an orthogonal solution.
https://github.com/knz/cockroach/blob/20191129-rfc-halloween/docs/RFCS/20191014_halloween.md
There is no new idea in this text (I copy-pasted the text from the other RFC) and the implementation PRs are already out, so if the reviewers are satisfied I would gladly already merge the RFC as in-progress or completed.