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's GC keeper with specific snapshot does not work with TiDB v6.1 or below #834

Closed
kennytm opened this issue Dec 19, 2024 · 1 comment · Fixed by #835
Closed

Comments

@kennytm
Copy link
Contributor

kennytm commented Dec 19, 2024

Bug Report

Please answer these questions before submitting your issue. Thanks!

  1. What did you do?

Use TiDB as a data-source of sync-diff-inspector.
Set snapshot config to some fixed value.
Inspect the Service GC Safepoint via pd-ctl service-gc-safepoint

  1. What did you expect to see?

The service-gc-safepoint list contains Sync_diff_xxxxxxxxx.

  1. What did you see instead?

The safepoint is not registered.

The sync-diff log suggested the safepoint is 0.

[2024/12/19 16:32:30.080 +08:00] [INFO] [pd.go:228] ["generate dumpling gc safePoint id"] [id=Sync_diff_1734597150080542000]
[2024/12/19 16:32:30.080 +08:00] [DEBUG] [pd.go:230] ["update PD safePoint limit with ttl"] [safePoint=0] [updateInterval=2m30s]
  1. What version of TiDB are you using (tidb-server -V or run select tidb_version(); on TiDB)?

v5.4.1, v6.1.7

  1. which tool are you using?

sync-diff-inspector

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

v6.5.3

@kennytm
Copy link
Contributor Author

kennytm commented Dec 19, 2024

if pdCli != nil {
// Get latest snapshot
latestSnap, err := utils.GetSnapshot(ctx, db)
if err != nil {
log.Info("failed to get snapshot, user should guarantee the GC stopped during diff progress.")
return
}
if len(latestSnap) == 1 {
if len(snap) == 0 {
snap = latestSnap[0]
}
// compare the snapshot and choose the small one to lock
if strings.Compare(latestSnap[0], snap) < 0 {
snap = latestSnap[0]
}
}
err = utils.StartGCSavepointUpdateService(ctx, pdCli, db, snap)
if err != nil {
log.Info("failed to keep snapshot, user should guarantee the GC stopped during diff progress.")
} else {
log.Info("start update service to keep GC stopped automatically")
}
}

Sync-diff-inspector determines the safepointTS using the following method:

  1. Use SHOW MASTER STATUS to get the cluster's current TSO.
  2. Compare with the given data-sources.*.snapshot, and use the earlier one (minimum).
  3. Set as the safe point TS.

Unfortunately, on TiDB v6.1 and before, there is a bug in SHOW MASTER STATUS that when @@tidb_snapshot is defined it will always return 0:

$ # v6.1.7
$ mysql -u root -h 127.1 -P 4000 -e 'set tidb_snapshot = now(); show master status'             
+-------------+----------+--------------+------------------+-------------------+
| File        | Position | Binlog_Do_DB | Binlog_Ignore_DB | Executed_Gtid_Set |
+-------------+----------+--------------+------------------+-------------------+
| tidb-binlog |        0 |              |                  |                   |
+-------------+----------+--------------+------------------+-------------------+

$ # v6.5.3
$ mysql -u root -h 127.1 -P 4000 -e 'set tidb_snapshot = now(); show master status;'
+-------------+--------------------+--------------+------------------+-------------------+
| File        | Position           | Binlog_Do_DB | Binlog_Ignore_DB | Executed_Gtid_Set |
+-------------+--------------------+--------------+------------------+-------------------+
| tidb-binlog | 454714341457920000 |              |                  |                   |
+-------------+--------------------+--------------+------------------+-------------------+

Since 0 is smaller than any given value, sync-diff-inspector will pick this as the safepoint and issued an invalid request to PD which is ignored.

We are not going to figure out how to fix TiDB. But sync-diff-inspector should prepare for a zero reply from SHOW MASTER STATUS and treat this as invalid.

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

Successfully merging a pull request may close this issue.

1 participant