search

pingcap / tidb-tools

tidb-tools are some useful tool collections for TiDB.
Apache License 2.0
286 stars 191 forks source link

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

Open kennytm opened 1 year ago

kennytm commented 1 year ago

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:

https://github.com/pingcap/tidb-tools/blob/00998a9a4bfd08eb63449186d147b0e6bb1fea5d/sync_diff_inspector/utils/pd.go#L180

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 commented 1 year ago

The problem should still exist on the master version.

https://github.com/pingcap/tidb-tools/blob/45729968487c9b4aa5820b3c00326be0c9bb89d5/sync_diff_inspector/utils/pd.go#L173-L178

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

https://github.com/pingcap/tidb-tools/blob/45729968487c9b4aa5820b3c00326be0c9bb89d5/sync_diff_inspector/utils/pd.go#L43

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.