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

txn: disable amend by default on master #21829

Merged
merged 4 commits into from
Dec 16, 2020

Conversation

cfzjywxk
Copy link
Contributor

@cfzjywxk cfzjywxk commented Dec 16, 2020

What problem does this PR solve?

Problem Summary:

Transaction amend is not a released feature on the master branch, disable it by default. And there is still more new compatibility issues to solve like #21470.

What is changed and how it works?

What's Changed:

How it Works:

Related changes

Check List

  • Unit test

Side effects

Release note

  • No release note

@cfzjywxk cfzjywxk added the sig/transaction SIG:Transaction label Dec 16, 2020
@github-actions github-actions bot added the sig/sql-infra SIG: SQL Infra label Dec 16, 2020
@sticnarf
Copy link
Contributor

sticnarf commented Dec 16, 2020

Maybe we should also change the way to solve the schema change issue for async commit. Async commit depends on "amend" to deal with schema issues.

Copy link
Member

@jackysp jackysp left a comment

Choose a reason for hiding this comment

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

LGTM

@ti-srebot ti-srebot added the status/LGT1 Indicates that a PR has LGTM 1. label Dec 16, 2020
@cfzjywxk
Copy link
Contributor Author

Maybe we should also change the way to solve the schema change issue for async commit.

Any new ideas by now?

@sticnarf
Copy link
Contributor

Maybe we should also change the way to solve the schema change issue for async commit.

Any new ideas by now?

Use MDL-like reference counting (which we will probably implement) to control async commit too.

@sticnarf
Copy link
Contributor

LGTM

@ti-srebot ti-srebot removed the status/LGT1 Indicates that a PR has LGTM 1. label Dec 16, 2020
@ti-srebot ti-srebot added the status/LGT2 Indicates that a PR has LGTM 2. label Dec 16, 2020
@cfzjywxk
Copy link
Contributor Author

/merge

@ti-srebot ti-srebot added the status/can-merge Indicates a PR has been approved by a committer. label Dec 16, 2020
@ti-srebot
Copy link
Contributor

/run-all-tests

@ti-srebot ti-srebot merged commit 291d0b8 into pingcap:master Dec 16, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
sig/sql-infra SIG: SQL Infra sig/transaction SIG:Transaction status/can-merge Indicates a PR has been approved by a committer. status/LGT2 Indicates that a PR has LGTM 2.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants