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

domain: stop schema validator to prohibit DML executing when tidb lost session of etcd #8441

Merged
merged 14 commits into from Nov 28, 2018

Conversation

@winkyao
Member

winkyao commented Nov 25, 2018

What problem does this PR solve?

The etcd is responsible for schema synchronization, we should ensure there is at most two diffrent schema version in the TiDB cluster, to make the data/schema be consistent.

If we lost connection/session to etcd, the cluster will treats this TiDB as a down instance, and etcd will remove the key of /tidb/ddl/all_schema_versions/tidb-id.

Say the schema version now is 1, the owner is changing the schema version to 2, it will not wait for this down TiDB syncing the schema, then continue to change the TiDB schema to version 3. Unfortunately, this down TiDB schema version will still be version 1. And version 1 is not consistent to version 3.

So we need to stop the schema validator to prohibit the DML executing when we lost session of etcd.

This PR revert partial changes of #7810


This change is Reviewable

@winkyao

This comment has been minimized.

Member

winkyao commented Nov 25, 2018

@tiancaiamao

This comment has been minimized.

Contributor

tiancaiamao commented Nov 25, 2018

it will not wait for this down TiDB syncing the schema, then continue to change the TiDB schema to version 3

A schema change should meet either of the two conditions:

  1. There is no network partition, all peers agree with a consistent cluster state
  2. Wait a lease since last change, to guarantee that a schema won't change twice within a lease

If none of the condition is satisfied, how could it change to a newer version 3 ?

So we need to stop the schema validator to prohibit the DML executing when we lost session of etcd.

We do not need to stop the schema validator, instead, we should stop the version change in the owner. @winkyao

@shenli

This comment has been minimized.

Member

shenli commented Nov 25, 2018

@winkyao Please add a test case.

@winkyao winkyao added the status/WIP label Nov 26, 2018

@winkyao

This comment has been minimized.

Member

winkyao commented Nov 26, 2018

I'll add test cases.

@winkyao winkyao removed the status/WIP label Nov 26, 2018

@winkyao

This comment has been minimized.

Member

winkyao commented Nov 26, 2018

@tiancaiamao It is another issue, we can fix the issue you mentioned later, but not his PR.

@winkyao winkyao force-pushed the winkyao:fix_schema_validator branch from e19a768 to 3f64fd0 Nov 26, 2018

winkyao added some commits Nov 26, 2018

Show resolved Hide resolved ddl/fail_db_test.go Outdated
Show resolved Hide resolved domain/domain.go Outdated
Show resolved Hide resolved domain/domain.go Outdated
Show resolved Hide resolved domain/schema_validator.go

winkyao added some commits Nov 26, 2018

Show resolved Hide resolved domain/schema_validator.go
Show resolved Hide resolved session/session.go Outdated

winkyao added some commits Nov 26, 2018

@tiancaiamao

This comment has been minimized.

Contributor

tiancaiamao commented Nov 26, 2018

LGTM

@winkyao

This comment has been minimized.

Member

winkyao commented Nov 27, 2018

@winkyao

This comment has been minimized.

Member

winkyao commented Nov 27, 2018

/run-all-tests

@winkyao

This comment has been minimized.

Member

winkyao commented Nov 27, 2018

/run-common-test tidb-test=pr/668

@winkyao

This comment has been minimized.

Member

winkyao commented Nov 27, 2018

/run-integration-common-test tidb-test=pr/668

@crazycs520 crazycs520 added status/LGT2 and removed status/LGT1 labels Nov 27, 2018

@ciscoxll

LGTM

@winkyao winkyao added require-LGT3 and removed status/LGT2 labels Nov 28, 2018

@winkyao winkyao merged commit 419c881 into pingcap:master Nov 28, 2018

4 checks passed

ci/circleci Your tests passed on CircleCI!
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
jenkins-ci-tidb/build Jenkins job succeeded.
Details
license/cla Contributor License Agreement is signed.
Details

@winkyao winkyao deleted the winkyao:fix_schema_validator branch Nov 28, 2018

winkyao added a commit to winkyao/tidb that referenced this pull request Nov 28, 2018

winkyao added a commit to winkyao/tidb that referenced this pull request Nov 28, 2018

winkyao added a commit to winkyao/tidb that referenced this pull request Nov 28, 2018

zz-jason added a commit that referenced this pull request Nov 28, 2018

zz-jason added a commit that referenced this pull request Nov 28, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment