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

sync-diff-inspector panics with nonstandard custom server-version in TiDB Server #719

Open
kennytm opened this issue Apr 11, 2023 · 1 comment

Comments

@kennytm
Copy link
Contributor

kennytm commented Apr 11, 2023

Bug Report

Please answer these questions before submitting your issue. Thanks!

1. What did you do?

  1. Set the server-version of a TiDB server to 99.7.25-TiDB-v6.1
  2. Run sync-diff-inspector with this server involved

2. What did you expect to see?

Sync-diff-inspector completes successfully.

3. What did you see instead?

Panicked on this line:

versionStr = tidbVersionRegex.FindString(versionStr)[1:]

4. What version of TiDB are you using (tidb-server -V or run select tidb_version(); on TiDB)?

5. which tool are you using?

sync-diff-inspector

6. what versionof tool are you using (pump -V or tidb-lightning -V or syncer -V)?

v6.1.1

@kennytm
Copy link
Contributor Author

kennytm commented Apr 11, 2023

The problem should still exist on the master version.

// parse versino string to semver.Version
func parseVersion(versionStr string) (*semver.Version, error) {
versionStr = tidbVersionRegex.FindString(versionStr)[1:]
versionStr = strings.TrimPrefix(versionStr, "v")
return semver.NewVersion(versionStr)
}

In our example, the customized version string only has 2 components ("-v6.1"), which does not match the required regexp

tidbVersionRegex = regexp.MustCompile(`-[v]?\d+\.\d+\.\d+([0-9A-Za-z-]+(\.[0-9A-Za-z-]+)*)?`)

so tidbVersionRegex.FindString(versionStr) will return an empty slice and taking [1:] will crash the program.

Since this function is dealing with GC Safepoint with the PD API, it should ask PD for the version rather than TiDB. Or simply run UpdateServiceGCSafePoint directly and fallback to "unsupported" once we get codes.Unimplemented.

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

No branches or pull requests

1 participant