pingcap / pingcap/tidb

Consider disabling --checksum for BR backup

Open
#43,007 4 comments 1 reaction 0 assignees View on GitHub
component/br type/feature-request
Dominant language
Go
Stars
40.5k
Forks
6.2k
PR merge metrics
PR metrics pending

Description

## Feature Request

**Is your feature request related to a problem? Please describe:**

Currently BR backup performs checksum twice, once during SST generation in TiKV, and once again after a table is backed up using TiKV Coprocessor. These two checksums are compared to verify correctness.

However, these checksums are calculated from the exact same source: the online TiKV database. The actual uploaded SST file could be corrupt but this won't be detected by the checksum.

The only way they differ is if the RocksDB snapshot mechanism break down or the checksum algorithm is changed. These are not something the users really care about. That is to say, running with `--checksum=1` only wasted CPU on TiKV to prove something that can never be false.

**Describe the feature you'd like:**

1. Given that TiKV always recorded the checksum into `backupmeta` regardless of the `--checksum` flag, we can safely change the flag's default value to `false`.
2. Enhance the `br debug checksum` command to actually read the SST files to calculate that file's CRC64XOR etc. Currently it only compares the SHA256 hash.

In the documentation recommend users to run `br debug checksum` after performing `br backup` to ensure the integrity of the uploaded backup archive.

> We are only talking about BR backup here. The default `--checksum` setting for BR *restore* should still and always be `true`.

**Describe alternatives you've considered:**

A. Change the `--checksum` flag to mean actually performing `br debug checksum`. Note that `br debug checksum` will need to download the SST files again so it will double the I/O cost of the target cloud storage.

B. Don't calculate CRC64XOR, keeping the current implementation of `br debug checksum`. Checking SHA256 should be sufficient to ensure the file content is intact.

**Teachability, Documentation, Adoption, Migration Strategy:**

For the public API since we are only changing the default value of a flag this is fully compatible.

The `br debug` command is currently hidden and `br debug checksum` doubly so. They need to be revealed and documented.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.