tidb icon indicating copy to clipboard operation
tidb copied to clipboard

BR-restore should still perform checksum even when the schema checksum is missing

Open kennytm opened this issue 1 year ago • 4 comments

Bug Report

Please answer these questions before submitting your issue. Thanks!

1. Minimal reproduce step (Required)

  1. Perform br backup full --checksum=0
  2. Perform br restore full --checksum=1

2. What did you expect to see? (Required)

BR restore compare the ADMIN CHECKSUM with the sum of crc64xor of all files, which are always computed in br backup regardless of original --checksum.

3. What did you see instead (Required)

BR restore does not compare the checksum because it used the Schema's crc64xor which are not populated without --checksum=1.

4. What is your TiDB version? (Required)

v6.5 and above

kennytm avatar Sep 27 '24 08:09 kennytm

Split off from https://github.com/pingcap/tidb/issues/43007#issuecomment-2373481127.

Since that the table schema's crc64xor is just the sum of the table's data files, the schema checksum is redundant. BR restore could simply rely on the total of the file crc64xor. I suggest

  1. Create a br debug command to populate the table schemas' crc64xor fields, so the backupmeta can be used with br restore full --checksum=1 without upgrading BR (which has to match the target cluster version)
  2. Adjust BR restore to operate on the sum of file crc64xor, so --checksum=1 always works. If the schema crc64xor exists, do an additional check to ensure the two are equivalent.

kennytm avatar Sep 27 '24 08:09 kennytm

/assign @Tristan1900

BornChanger avatar Oct 15 '24 09:10 BornChanger

@BornChanger: GitHub didn't allow me to assign the following users: Tristan1900.

Note that only pingcap members with read permissions, repo collaborators and people who have commented on this issue/PR can be assigned. Additionally, issues/PRs can only have 10 assignees at the same time. For more information please see the contributor guide

In response to this:

/assign @Tristan1900

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

ti-chi-bot[bot] avatar Oct 15 '24 09:10 ti-chi-bot[bot]

After this fixing this issue I feel like we need to adjust the semantics of the --checksum flag and maybe hide it for backup, looks like when it enabled the only extra work it does is to compare the checksum of files from BR side to the original one in cop tikv, which is not recommended anymore per team's previous discussion.

Tristan1900 avatar Oct 15 '24 17:10 Tristan1900