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

*: Reduce data inconsistencies #31547

Merged
merged 28 commits into from
Feb 11, 2022
Merged

*: Reduce data inconsistencies #31547

merged 28 commits into from
Feb 11, 2022

Conversation

ekexium
Copy link
Contributor

@ekexium ekexium commented Jan 11, 2022

What problem does this PR solve?

Issue Number: ref #26833

Problem Summary:

Defend against data inconsistency and improve its troubleshooting.
It's developed in a separate feature branch ft-data-inconsistency.
PRs can be found in the tracking issue #26833.

What is changed and how it works?

Two features: the mutation checker and the assertion, and error message improvements.

Check List

Tests

  • Unit test
  • Integration test
  • Manual test (add detailed scripts or steps below)
  • No code

Side effects

  • Performance regression: Consumes more CPU
  • Performance regression: Consumes more Memory
  • Breaking backward compatibility

Documentation

  • Affects user behaviors
  • Contains syntax changes
  • Contains variable changes
  • Contains experimental features
  • Changes MySQL compatibility

Release note

Changed error messages for data-index inconsistency errors.
Add new error types for data-index inconsistency errors

lysu and others added 16 commits September 13, 2021 18:13
* feat: check index key in AddRecord

Signed-off-by: ekexium <ekexium@gmail.com>

* feat: check for AddRecord/UpdateRecord

Signed-off-by: ekexium <ekexium@gmail.com>

* ignore deletion of indices if NeedRestoredData

Signed-off-by: ekexium <ekexium@gmail.com>

* check values of row mutations

Signed-off-by: ekexium <ekexium@gmail.com>

* style: fix naming

Signed-off-by: ekexium <ekexium@gmail.com>

* skip when sh == 0

Signed-off-by: ekexium <ekexium@gmail.com>

* check in RemoveRecord

Signed-off-by: ekexium <ekexium@gmail.com>

* modify license header

Signed-off-by: ekexium <ekexium@gmail.com>

* test: add unit tests

Signed-off-by: ekexium <ekexium@gmail.com>

* add lisence header

Signed-off-by: ekexium <ekexium@gmail.com>

* also truncate decoded mutation

Signed-off-by: ekexium <ekexium@gmail.com>

* tidy up

Signed-off-by: ekexium <ekexium@gmail.com>

* refactor according to comments

Signed-off-by: ekexium <ekexium@gmail.com>

Auto stash before rebase of "ft-data-inconsistency"

* skip partitioned table

Signed-off-by: ekexium <ekexium@gmail.com>

* save columnMap in stmtctx

* skip when sh == 0

Some implementations of MemBuffer doesn't support staging. We don't care about them for now

* reuse a slice in a loop

* save reusable maps in stmtctx

* save reusable maps in txn option

* add unit test for checkIndexKeys

* address comments in test
* check handle consistency

Signed-off-by: ekexium <ekexium@gmail.com>

* refactor according to comments

Signed-off-by: ekexium <ekexium@gmail.com>

* refactor according to comments

Signed-off-by: ekexium <ekexium@gmail.com>
* add a sysvar to enable mutation checker

Signed-off-by: ekexium <ekexium@gmail.com>

* add an HTTP API to modify tidb_enable_mutation_checker

Signed-off-by: ekexium <ekexium@gmail.com>

* hide tidb_enable_mutation_checker

Signed-off-by: ekexium <ekexium@gmail.com>

* change the default behavior: only enable mutation checker for new clusters

Signed-off-by: ekexium <ekexium@gmail.com>

* fix name

Signed-off-by: ekexium <ekexium@gmail.com>
* close #29469; fix: mistakenly shadowed name idx

Signed-off-by: ekexium <ekexium@gmail.com>

* test admin check table when uniqueness is violated

Signed-off-by: ekexium <ekexium@gmail.com>
Signed-off-by: ekexium <ekexium@gmail.com>
Signed-off-by: ekexium <ekexium@gmail.com>

Co-authored-by: Ti Chi Robot <ti-community-prow-bot@tidb.io>
Signed-off-by: ekexium <ekexium@gmail.com>
* fix admin test

Signed-off-by: ekexium <ekexium@gmail.com>

* fix gosec

Signed-off-by: ekexium <ekexium@gmail.com>

* satisfy gosec

Signed-off-by: ekexium <ekexium@gmail.com>
* Pick unistore changes from #28313, ref #26833

* fix build

Co-authored-by: MyonKeminta <MyonKeminta@users.noreply.github.com>
* fix admin test

Signed-off-by: ekexium <ekexium@gmail.com>

* fix gosec

Signed-off-by: ekexium <ekexium@gmail.com>

* satisfy gosec

Signed-off-by: ekexium <ekexium@gmail.com>

* add error codes for mutation checker; redact log if needed

Signed-off-by: ekexium <ekexium@gmail.com>

* add comments

Signed-off-by: ekexium <ekexium@gmail.com>

* fix errors.toml

Signed-off-by: ekexium <ekexium@gmail.com>

* better redact log

Signed-off-by: ekexium <ekexium@gmail.com>

* address comments

Signed-off-by: ekexium <ekexium@gmail.com>

* fix argument order

Signed-off-by: ekexium <ekexium@gmail.com>

* log error using redacted error object

Signed-off-by: ekexium <ekexium@gmail.com>

* modify log message

Signed-off-by: ekexium <ekexium@gmail.com>
* *: set consistent assertion for DML

* table: check duplicate for row_id

* update client-go

* fix fail in br

* update client-go, kvproto and add failpoints

* update kvproto and client-go

* update client-go for metrics

* Support assertion with existance state fetched during pessimistic lock phase

* Fix incorrect CheckExistence implementation in unistore

* support assertion level

* Fix the bug that untouched index has wrong assertion

* Remove change to explaintest script

* Implement assertion in unistore

* Add a basic test case for assertion

* Add tests to SetAssertion interface

* Add test case for previsously found bug

* update client-go

* Fix tests

* Fix store not closed in TestSetAssertion

* Address comments

* Fix ignore assertion in CreateIndex overwritten

* Fix build

* Test clustered index

* fix fmt

* Enable assertion only for new clusters

Co-authored-by: lysu <sulifx@gmail.com>
Co-authored-by: MyonKeminta <MyonKeminta@users.noreply.github.com>
Co-authored-by: Ti Chi Robot <ti-community-prow-bot@tidb.io>
…ogs (#30897)

Co-authored-by: MyonKeminta <MyonKeminta@users.noreply.github.com>
Signed-off-by: ekexium <ekexium@gmail.com>
@ti-chi-bot
Copy link
Member

ti-chi-bot commented Jan 11, 2022

[REVIEW NOTIFICATION]

This pull request has been approved by:

  • MyonKeminta
  • cfzjywxk

To complete the pull request process, please ask the reviewers in the list to review by filling /cc @reviewer in the comment.
After your PR has acquired the required number of LGTMs, you can assign this pull request to the committer in the list by filling /assign @committer in the comment to help you merge this pull request.

The full list of commands accepted by this bot can be found here.

Reviewer can indicate their review by submitting an approval review.
Reviewer can cancel approval by submitting a request changes review.

@ti-chi-bot ti-chi-bot added release-note Denotes a PR that will be considered when it comes time to generate release notes. do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Jan 11, 2022
Signed-off-by: ekexium <ekexium@gmail.com>
* fix build

Signed-off-by: ekexium <ekexium@gmail.com>

* fix test

Signed-off-by: ekexium <ekexium@gmail.com>
@ekexium
Copy link
Contributor Author

ekexium commented Jan 11, 2022

/run-unit-test
/run-check_dev_2

@ekexium
Copy link
Contributor Author

ekexium commented Jan 12, 2022

/run-unit-test

1 similar comment
@ekexium
Copy link
Contributor Author

ekexium commented Jan 12, 2022

/run-unit-test

* add a error code for assertion

Signed-off-by: ekexium <ekexium@gmail.com>

* take care of log redaction

Signed-off-by: ekexium <ekexium@gmail.com>

* generate errors.toml

Signed-off-by: ekexium <ekexium@gmail.com>

* get mvcc history

Signed-off-by: ekexium <ekexium@gmail.com>

* refactor according to comments

Signed-off-by: ekexium <ekexium@gmail.com>

* use ctx logger

Signed-off-by: ekexium <ekexium@gmail.com>
@sre-bot
Copy link
Contributor

sre-bot commented Jan 18, 2022

MyonKeminta and others added 2 commits January 19, 2022 13:34
Co-authored-by: MyonKeminta <MyonKeminta@users.noreply.github.com>
* Fix unstable test TestCheckFailReport

* Update executor/distsql.go

Co-authored-by: Ziqian Qin <ekexium@gmail.com>

Co-authored-by: MyonKeminta <MyonKeminta@users.noreply.github.com>
Co-authored-by: Ziqian Qin <ekexium@gmail.com>
@ekexium
Copy link
Contributor Author

ekexium commented Jan 19, 2022

/run-unit-test

store/driver/txn/txn_driver.go Outdated Show resolved Hide resolved
table/tables/tables.go Show resolved Hide resolved
table/tables/tables_test.go Outdated Show resolved Hide resolved
Signed-off-by: ekexium <ekexium@gmail.com>
@ti-chi-bot ti-chi-bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jan 25, 2022
Co-authored-by: MyonKeminta <MyonKeminta@users.noreply.github.com>
@ti-chi-bot ti-chi-bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jan 26, 2022
Co-authored-by: MyonKeminta <MyonKeminta@users.noreply.github.com>
@ti-chi-bot ti-chi-bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Feb 9, 2022
@ti-chi-bot ti-chi-bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Feb 10, 2022
Signed-off-by: ekexium <ekexium@gmail.com>
@ti-chi-bot ti-chi-bot added the status/LGT1 Indicates that a PR has LGTM 1. label Feb 11, 2022
@ti-chi-bot ti-chi-bot added status/LGT2 Indicates that a PR has LGTM 2. and removed status/LGT1 Indicates that a PR has LGTM 1. labels Feb 11, 2022
@ekexium
Copy link
Contributor Author

ekexium commented Feb 11, 2022

/merge

@ti-chi-bot
Copy link
Member

@ekexium: /merge is only allowed for the committers, you can assign this pull request to the committer in list by filling /assign @committer in the comment to help merge this pull request.

In response to this:

/merge

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the ti-community-infra/tichi repository.

@cfzjywxk
Copy link
Contributor

/merge

@ti-chi-bot
Copy link
Member

This pull request has been accepted and is ready to merge.

Commit hash: b706ea3

@ti-chi-bot ti-chi-bot added the status/can-merge Indicates a PR has been approved by a committer. label Feb 11, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release-note Denotes a PR that will be considered when it comes time to generate release notes. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. 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.

6 participants