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

owner: revoke on ctx.Done. #4624

Merged
merged 6 commits into from Sep 26, 2017

Conversation

Projects
None yet
3 participants
@coocood
Member

coocood commented Sep 25, 2017

Without revoke, when bootstrap finished or server exit, there is an owner entry left on etcd, so the new DDL cannot become the owner instantly.

The test depends on etcd, so I just did a manual test to verify that initialize cluster and restart the tidb-server can create table instantly.

owner: resign on ctx.Done.
The test depends on etcd, so I just did manual test to verify that initialize cluster and restart the tidb-server
can create table instantly.
@coocood

This comment has been minimized.

Show comment
Hide comment
@coocood
Member

coocood commented Sep 25, 2017

@zimulala

This comment has been minimized.

Show comment
Hide comment
@zimulala

zimulala Sep 25, 2017

Member

Please add some tests in ddl_etcd_test.

Member

zimulala commented Sep 25, 2017

Please add some tests in ddl_etcd_test.

@coocood coocood changed the title from owner: resign on ctx.Done. to owner: revoke on ctx.Done. Sep 25, 2017

@coocood

This comment has been minimized.

Show comment
Hide comment
@coocood
Member

coocood commented Sep 25, 2017

@zimulala PTAL

time.Duration(ManagerSessionTTL)*time.Second)
_, err := m.etcdCli.Revoke(cancelCtx, leaseID)
cancel()
log.Infof("%s break campaign loop, revoke err %v", logPrefix, err)

This comment has been minimized.

@shenli

shenli Sep 25, 2017

Member

If err is nil, we do not need print it.

@shenli

shenli Sep 25, 2017

Member

If err is nil, we do not need print it.

This comment has been minimized.

@coocood

coocood Sep 25, 2017

Member

The old code prints the log for breaking campaign loop, not only for the error.

@coocood

coocood Sep 25, 2017

Member

The old code prints the log for breaking campaign loop, not only for the error.

@zimulala

This comment has been minimized.

Show comment
Hide comment
@zimulala

zimulala Sep 25, 2017

Member

LGTM

Member

zimulala commented Sep 25, 2017

LGTM

@shenli

This comment has been minimized.

Show comment
Hide comment
@shenli

shenli Sep 25, 2017

Member

LGTM

Member

shenli commented Sep 25, 2017

LGTM

@shenli

shenli approved these changes Sep 25, 2017

@shenli

This comment has been minimized.

Show comment
Hide comment
@shenli

shenli Sep 25, 2017

Member

/run-all-tests

Member

shenli commented Sep 25, 2017

/run-all-tests

@coocood coocood merged commit e19e1ac into master Sep 26, 2017

10 of 11 checks passed

jenkins-ci-tidb/mybatis-test Jenkins job failed
Details
ci/circleci Your tests passed on CircleCI!
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls First build on master at 72.72%
Details
jenkins-ci-tidb/build Jenkins job succeeded.
Details
jenkins-ci-tidb/common-test Jenkins job succeeded.
Details
jenkins-ci-tidb/integration-common-test Jenkins job succeeded.
Details
jenkins-ci-tidb/integration-ddl-test Jenkins job succeeded.
Details
jenkins-ci-tidb/sqllogic-test Jenkins job succeeded.
Details
jenkins-ci-tidb/unit-test Jenkins job succeeded.
Details
license/cla Contributor License Agreement is signed.
Details

@coocood coocood deleted the coocood/resign-on-done branch Sep 26, 2017

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