Skip to content

Add a new key error type AssertionFailed#815

Merged
disksing merged 3 commits intopingcap:masterfrom
MyonKeminta:m/add-assertion-fail
Oct 11, 2021
Merged

Add a new key error type AssertionFailed#815
disksing merged 3 commits intopingcap:masterfrom
MyonKeminta:m/add-assertion-fail

Conversation

@MyonKeminta
Copy link
Contributor

This might be part of pingcap/tidb#26833

@disksing
Copy link
Contributor

disksing commented Oct 9, 2021

Please take a look @ekexium @sticnarf

@sticnarf
Copy link
Contributor

sticnarf commented Oct 9, 2021

LGTM while I think we can keep it until the related PR is ready to merge, in case there are still some small changes.

Copy link
Member

@ekexium ekexium left a comment

Choose a reason for hiding this comment

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

LGTM

Comment on lines +766 to +767
uint64 existed_start_ts = 4;
uint64 existed_commit_ts = 5;
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
uint64 existed_start_ts = 4;
uint64 existed_commit_ts = 5;
uint64 existing_start_ts = 4;
uint64 existing_commit_ts = 5;

CommitTsExpired commit_ts_expired = 7; // Commit ts is earlier than min commit ts of a transaction.
TxnNotFound txn_not_found = 8; // Txn not found when checking txn status.
CommitTsTooLarge commit_ts_too_large = 9; // Calculated commit TS exceeds the limit given by the user.
AssertionFailed assertion_failed = 10; // Assertion of a `Mutation` is evaluated to fail.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
AssertionFailed assertion_failed = 10; // Assertion of a `Mutation` is evaluated to fail.
AssertionFailed assertion_failed = 10; // Assertion of a `Mutation` is evaluated as a failure.

@disksing
Copy link
Contributor

@MyonKeminta want to address comments?

@MyonKeminta
Copy link
Contributor Author

@MyonKeminta want to address comments?

Sorry I wasn't working on this for some time. I'll do it now

@disksing disksing merged commit d957056 into pingcap:master Oct 11, 2021
@peng1999 peng1999 mentioned this pull request Oct 11, 2021
@MyonKeminta MyonKeminta deleted the m/add-assertion-fail branch October 11, 2021 09:38
@MyonKeminta MyonKeminta restored the m/add-assertion-fail branch October 11, 2021 10:12
@MyonKeminta
Copy link
Contributor Author

🤔 Actually this PR don't need to be merged so early...

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.

4 participants