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

store/tikv: handle the error of "mismatch cluster id" #7053

Merged
merged 3 commits into from
Jul 16, 2018

Conversation

zimulala
Copy link
Contributor

@zimulala zimulala commented Jul 13, 2018

What have you changed? (mandatory)

We will exit the current TiDB when we encounter the error of "mismatch cluster id, need 6577643568868929567 but got 6577623361857143480".
The TiDB will meet the error of “mismatch cluster id” when the cluster ID of the TiDB‘s PD client does not match the PD.

What is the type of the changes? (mandatory)

  • Improvement (non-breaking change which is an improvement to an existing feature)

How has this PR been tested? (mandatory)

In local

Does this PR affect documentation (docs/docs-cn) update? (mandatory)

No

Does this PR affect tidb-ansible update? (mandatory)

No

Does this PR need to be added to the release notes? (mandatory)

release note:
Exit the current TiDB when it encounters the error of "mismatch cluster id".

@@ -233,6 +234,9 @@ func (b *Backoffer) Backoff(typ backoffType, err error) error {
}
}
log.Warn(errMsg)
if strings.Contains(errMsg, ErrMismatchClusterID.Error()) {
Copy link
Member

Choose a reason for hiding this comment

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

Can we define a string constant for "mismatch cluster id"?

Copy link
Member

Choose a reason for hiding this comment

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

Is it better to check and handle this in the pd-client? @disksing What do you think about it?

Copy link
Contributor

Choose a reason for hiding this comment

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

I think we can check it at the beginning of this function.

Copy link
Member

Choose a reason for hiding this comment

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

Good idea.

@@ -22,6 +22,8 @@ import (
var (
// ErrBodyMissing response body is missing error
ErrBodyMissing = errors.New("response body is missing")
// ErrMismatchClusterID returns the error that the cluster ID of the PD client does not match the PD.
ErrMismatchClusterID = errors.New("mismatch cluster id")
Copy link
Member

Choose a reason for hiding this comment

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

We don't need to define an error if we never throw it.
I think just make the string constant and check the string will do.

Copy link
Member

Choose a reason for hiding this comment

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

Agree

@zimulala
Copy link
Contributor Author

PTAL @disksing @shenli @coocood

@disksing
Copy link
Contributor

LGTM

Copy link
Contributor

@ciscoxll ciscoxll left a comment

Choose a reason for hiding this comment

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

LGTM

@ciscoxll ciscoxll added the status/LGT2 Indicates that a PR has LGTM 2. label Jul 16, 2018
@ciscoxll
Copy link
Contributor

/run-all-tests

Copy link
Member

@jackysp jackysp left a comment

Choose a reason for hiding this comment

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

LGTM

@ciscoxll ciscoxll merged commit b729a60 into pingcap:master Jul 16, 2018
@zimulala zimulala deleted the mismatch-cluster-id branch August 7, 2018 06:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component/deployment status/LGT2 Indicates that a PR has LGTM 2.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants