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.

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

sql: deal with retriable errors when using a new txn #46829

Merged

Conversation

ajwerner
Copy link
Contributor

@ajwerner ajwerner commented Apr 1, 2020

In #46588 a bug was introduced when a retriable error was encountered while
using a new transaction for preparing. Prior to that commit, all error were
treated as not retriable. This was sort of a bummer. Retriable errors can
occur due to read within uncertainty. Before this PR, those retriable errors
would make their way to the client. Now we'll handle those retry errors
internally underneath connExecutor.prepare

Fixes #43251

Release note: None

@ajwerner ajwerner requested a review from andreimatei April 1, 2020 03:21
@cockroach-teamcity
Copy link
Member

This change is Reviewable

Copy link
Contributor

@andreimatei andreimatei left a comment

Choose a reason for hiding this comment

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

LGTM

Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained (waiting on @ajwerner and @andreimatei)


pkg/sql/conn_executor_test.go, line 635 at r1 (raw file):

}

// This test ensures that when in an explicit transaction and statement

implicit. And maybe put it in the test's name too.


pkg/sql/conn_executor_test.go, line 659 at r1 (raw file):

	testDB.Exec(t, "CREATE TABLE foo (i INT PRIMARY KEY)")

	stmt, err := sqlDB.Prepare("SELECT * FROM [SHOW COLUMNS FROM foo]")

make this statement simpler now that we have the knob

@ajwerner ajwerner force-pushed the ajwerner/fix-prepare-retry-error branch from 74cb251 to 4be0321 Compare April 1, 2020 16:51
In cockroachdb#46588 a bug was introduced when a retriable error was encountered while
using a new transaction for preparing. Prior to that commit, all error were
treated as not retriable. This was sort of a bummer. Retriable errors can
occur due to read within uncertainty. Before this PR, those retriable errors
would make their way to the client. Now we'll handle those retry errors
internally underneath `connExecutor.prepare`

Fixes cockroachdb#43251

Release note: None
@ajwerner ajwerner force-pushed the ajwerner/fix-prepare-retry-error branch from 4be0321 to 52653e6 Compare April 1, 2020 16:52
Copy link
Contributor Author

@ajwerner ajwerner 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=andreimatei

Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained (waiting on @andreimatei)


pkg/sql/conn_executor_test.go, line 635 at r1 (raw file):

Previously, andreimatei (Andrei Matei) wrote…

implicit. And maybe put it in the test's name too.

Done, this whole description was wrong.


pkg/sql/conn_executor_test.go, line 659 at r1 (raw file):

Previously, andreimatei (Andrei Matei) wrote…

make this statement simpler now that we have the knob

Done.

@craig
Copy link
Contributor

craig bot commented Apr 1, 2020

Canceled (will resume)

@craig
Copy link
Contributor

craig bot commented Apr 1, 2020

Build failed (retrying...)

@ajwerner
Copy link
Contributor Author

ajwerner commented Apr 1, 2020

bors r+

@craig
Copy link
Contributor

craig bot commented Apr 1, 2020

Build succeeded

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.

ccl/backupccl: TestBackupRestoreWithConcurrentWrites failed
3 participants