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

Add "START TRANSACTION WITH CAUSAL CONSISTENCY ONLY" #1162

Merged

Conversation

ekexium
Copy link
Contributor

@ekexium ekexium commented Jan 28, 2021

Signed-off-by: ekexium [email protected]

What problem does this PR solve?

See pingcap/tidb#22597

What is changed and how it works?

Check List

Tests

  • Unit test

Code changes

Side effects

Related changes

@CLAassistant
Copy link

CLAassistant commented Jan 28, 2021

CLA assistant check
All committers have signed the CLA.

@kennytm
Copy link
Contributor

kennytm commented Jan 28, 2021

if external consistency (> strict serializability) is the default and is here considered orthogonal to RU/RC/RR stuff, what happens if i do

START TRANSACTION /* WITH EXTERNAL CONSISTENCY */;
SET TRANSACTION ISOLATION LEVEL READ COMMITTED;
...

?

@ekexium
Copy link
Contributor Author

ekexium commented Jan 29, 2021

@kennytm External consistency doesn't mean strict serializability. It is orthogonal to isolation levels.

So I think it should not interfere with Read Committed.

TiDB used to guarantee external consistency. The new feature async commit can break it (for better performance), so we need a new grammar to control the behavior.

@sticnarf
Copy link

@kennytm Right... External consistency can be an ambiguous word.
Do you think we can use START TRANSACTION WITH CAUSAL CONSISTENCY?

@kennytm
Copy link
Contributor

kennytm commented Jan 29, 2021

So to clarify, is "external consistency" here the same concept as the one mentioned in https://cloud.google.com/spanner/docs/true-time-external-consistency and https://stackoverflow.com/questions/60365103/differences-between-strict-serializable-and-external-consistency (which both claimed it is stronger than linearizability + serializability)

@sticnarf
Copy link

So to clarify, is "external consistency" here the same concept as the one mentioned in https://cloud.google.com/spanner/docs/true-time-external-consistency and https://stackoverflow.com/questions/60365103/differences-between-strict-serializable-and-external-consistency (which both claimed it is stronger than linearizability + serializability)

No, the "external consistency" here is roughly the same with linearizability. It has nothing to do with serializability.

ast/misc.go Outdated Show resolved Hide resolved
parser.y Outdated Show resolved Hide resolved
@ti-srebot ti-srebot added the status/LGT1 LGT1 label Feb 3, 2021
@kennytm kennytm changed the title Add "start transaction without external consistency" Add "START TRANSACTION WITH CAUSAL CONSISTENCY ONLY" Feb 3, 2021
@kennytm
Copy link
Contributor

kennytm commented Feb 3, 2021

/lgtm

@ti-srebot ti-srebot added status/LGT2 LGT2 and removed status/LGT1 LGT1 labels Feb 3, 2021
@youjiali1995
Copy link
Contributor

/merge

@ti-srebot
Copy link
Contributor

/run-all-tests

@ti-srebot
Copy link
Contributor

@ekexium merge failed.

@youjiali1995
Copy link
Contributor

/merge

@ti-srebot
Copy link
Contributor

/run-all-tests

@ti-srebot
Copy link
Contributor

@ekexium merge failed.

@kennytm kennytm merged commit 32ef3e0 into pingcap:master Feb 3, 2021
tiancaiamao pushed a commit to tiancaiamao/parser that referenced this pull request Apr 27, 2021
* add a start transaction without external consistency statement

Signed-off-by: ekexium <[email protected]>

* change the grammar to 'with causal consistency'

Signed-off-by: ekexium <[email protected]>

* rename WithoutExternalConsistency to CausalConsistency

Signed-off-by: ekexium <[email protected]>

* change syntax to 'start transaction with causal consistency only'

Signed-off-by: ekexium <[email protected]>

Co-authored-by: ti-srebot <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants