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

remove read-only statement from transaction auto retry (#5026) #5051

Merged
merged 2 commits into from Nov 14, 2017

Conversation

wentaoxu
Copy link
Contributor

@wentaoxu wentaoxu commented Nov 9, 2017

  • *: Ignore readonly statement when retrying

  • remove explain statement from retry

  • remove all select statement from retry except It has setvar functions

  • remove execute statement from retry if it is read-only statement

@shenli
Copy link
Member

shenli commented Nov 9, 2017

LGTM

@winoros
Copy link
Member

winoros commented Nov 9, 2017

@wentaoxu please fix the error.

@wentaoxu wentaoxu closed this Nov 9, 2017
@wentaoxu wentaoxu reopened this Nov 10, 2017
@wentaoxu
Copy link
Contributor Author

@coocood @winoros

return &ExecStmt{
InfoSchema: infoSchema,
Plan: finalPlan,
Expensive: stmtCount(stmtNode, finalPlan, ctx.GetSessionVars().InRestrictedSQL),
Cacheable: plan.Cacheable(stmtNode),
Text: stmtNode.Text(),
ReadOnly: ast.IsReadOnly(readOnlyCheckStmt),
// this should not be 'plan.execute'
ReadOnly: ast.IsReadOnly(stmtNode),
Copy link
Member

Choose a reason for hiding this comment

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

It can be plan.Execute
There are two types of prepared statement binary and text.

@shenli
Copy link
Member

shenli commented Nov 13, 2017

@wentaoxu Any update?

wentaoxu and others added 2 commits November 14, 2017 11:16
* *: Ignore readonly statement when retrying

* remove explain statement from retry
* remove all select statement from retry except It has setvar functions
* remove execute statement from retry if it is read-only statement
@wentaoxu
Copy link
Contributor Author

wentaoxu commented Nov 14, 2017

PTAL @coocood, I get the prepared ast from session context and check if the ast is read-only. if plan cache merge into release-1.0, these codes can be deleted.

@coocood
Copy link
Member

coocood commented Nov 14, 2017

LGTM

@coocood
Copy link
Member

coocood commented Nov 14, 2017

/run-all-tests tikv=release-1.0 tidb-test=release-1.0

@wentaoxu
Copy link
Contributor Author

PTAL @zz-jason @winoros @shenli

Copy link
Member

@zz-jason zz-jason left a comment

Choose a reason for hiding this comment

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

LGTM

@shenli
Copy link
Member

shenli commented Nov 14, 2017

LGTM

@shenli shenli merged commit 12f6912 into pingcap:release-1.0 Nov 14, 2017
@sre-bot sre-bot added the contribution This PR is from a community contributor. label Dec 18, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
contribution This PR is from a community contributor.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants