sql: Fix "dangling rows" left over by schema change + rollback to savepoint #24888
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
…epoint Prior to this patch, the following sequence would write a row that's then unaccessible: begin; savepoint cockroach_restart; create table t(x int primary key); rollback to savepoint cockroach_restart; insert into t(x) values(1); release savepoint cockroach_restart; commit; The insert manages to resolve the table because we weren't clearing the "uncommitted descriptors" set that the session maintains. Since the table descriptor doesn't end up being committed, the row will be essentially unreachable. This patch is original work on the 1.1 branch. This bug is not present in the 2.0 release, where we're better about reseting state on transaction retries. I believe on 1.1 there's still a similar issue present when doing automatic retries (as opposed to user-directed ones through rollback to savepoint). I think fixing those would be more involved because it's less clear where to stick the cleanup done in this patch; I'd rather not do anything this point. Also note that, with the fix, the sequence above doesn't actually work. Instead, it deadlocks, just like it does on 2.0, because of cockroachdb#24885. However, the following works: begin; savepoint cockroach_restart; create table t(x int primary key); rollback to savepoint cockroach_restart; create table t(x int primary key); insert into t(x) values(1); release savepoint cockroach_restart; commit; Fixes cockroachdb#24785
|
This change is |
|
Review status: 0 of 2 files reviewed at latest revision, all discussions resolved. Comments from Reviewable |
|
bors r+ |
craig bot
pushed a commit
that referenced
this pull request
Apr 17, 2018
24888: sql: Fix "dangling rows" left over by schema change + rollback to savepoint r=andreimatei a=andreimatei Prior to this patch, the following sequence would write a row that's then unaccessible: begin; savepoint cockroach_restart; create table t(x int primary key); rollback to savepoint cockroach_restart; insert into t(x) values(1); release savepoint cockroach_restart; commit; The insert manages to resolve the table because we weren't clearing the "uncommitted descriptors" set that the session maintains. Since the table descriptor doesn't end up being committed, the row will be essentially unreachable. This patch is original work on the 1.1 branch. This bug is not present in the 2.0 release, where we're better about reseting state on transaction retries. I believe on 1.1 there's still a similar issue present when doing automatic retries (as opposed to user-directed ones through rollback to savepoint). I think fixing those would be more involved because it's less clear where to stick the cleanup done in this patch; I'd rather not do anything this point. Also note that, with the fix, the sequence above doesn't actually work. Instead, it deadlocks, just like it does on 2.0, because of #24885. However, the following works: begin; savepoint cockroach_restart; create table t(x int primary key); rollback to savepoint cockroach_restart; create table t(x int primary key); insert into t(x) values(1); release savepoint cockroach_restart; commit; Fixes #24785 cc @cockroachdb/release @gpaul Co-authored-by: Andrei Matei <andrei@cockroachlabs.com>
Build succeeded |
andreimatei
added a commit
to andreimatei/cockroach
that referenced
this pull request
Apr 18, 2018
Add a regression test for an old 1.1 bug. Test comes from cockroachdb#24888. Release note: None
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.
Prior to this patch, the following sequence would write a row that's
then unaccessible:
begin; savepoint cockroach_restart; create table t(x int primary key); rollback to savepoint cockroach_restart;
insert into t(x) values(1); release savepoint cockroach_restart; commit;
The insert manages to resolve the table because we weren't clearing the
"uncommitted descriptors" set that the session maintains. Since the
table descriptor doesn't end up being committed, the row will be
essentially unreachable.
This patch is original work on the 1.1 branch. This bug is not present
in the 2.0 release, where we're better about reseting state on
transaction retries.
I believe on 1.1 there's still a similar issue present when doing
automatic retries (as opposed to user-directed ones through rollback to
savepoint). I think fixing those would be more involved because it's
less clear where to stick the cleanup done in this patch; I'd rather not
do anything this point.
Also note that, with the fix, the sequence above doesn't actually work.
Instead, it deadlocks, just like it does on 2.0, because of #24885.
However, the following works:
begin; savepoint cockroach_restart; create table t(x int primary key); rollback to savepoint cockroach_restart;
create table t(x int primary key); insert into t(x) values(1); release savepoint cockroach_restart; commit;
Fixes #24785
cc @cockroachdb/release @gpaul