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

backup: hash backup file name #6177

Merged
merged 5 commits into from Dec 9, 2019
Merged

Conversation

3pointer
Copy link
Contributor

@3pointer 3pointer commented Dec 6, 2019

Signed-off-by: luancheng luancheng@pingcap.com

What have you changed?

use start key sha256 to replace raw start key, if we use start key as file name we could meet
"file name too long" error because some index key are longer than 255 bytes

What is the type of the changes?

  • Bugfix (a change which fixes an issue)

How is the PR tested?

Please select the tests that you ran to verify your changes:

  • Unit test
  • Integration test

Does this PR affect documentation (docs) or should it be mentioned in the release notes?

N/A

Does this PR affect tidb-ansible?

N/A

Refer to a related PR or issue link (optional)

Benchmark result if necessary (optional)

Any examples? (optional)

Comment on lines 574 to 579
"{}_{}_{}_{:?}",
store_id,
region.get_id(),
region.get_region_epoch().get_version(),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should use "{}", hex::encode(k).

Here is an example of {:?}, vec![1, 2, 3] => [1, 2, 3]

Copy link
Contributor Author

@3pointer 3pointer Dec 6, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hex::encode is used in sha256, I think use String::from_utf8 is enough

@@ -382,7 +382,7 @@ impl<E: Engine, R: RegionInfoProvider> Endpoint<E, R> {
let key = brange
.start_key
.clone()
.map(|k| hex::encode(k.into_raw().unwrap()));
.and_then(|k| tikv_util::file::sha256(&k.into_raw().unwrap()).ok());
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you add some comments about why we need it?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sure, add comment

@overvenus overvenus added component/backup-restore Component: backup, import, external_storage needs-cherry-pick-release-3.1 Type: Need cherry pick to release 3.1 labels Dec 6, 2019
@3pointer 3pointer force-pushed the fix_backup_filename branch 3 times, most recently from 4906ac6 to 6250fc5 Compare December 6, 2019 11:51
Signed-off-by: luancheng <luancheng@pingcap.com>
@3pointer 3pointer changed the title backup: hash backup file name WIP:backup: hash backup file name Dec 9, 2019
Signed-off-by: luancheng <luancheng@pingcap.com>
@3pointer 3pointer changed the title WIP:backup: hash backup file name backup: hash backup file name Dec 9, 2019
Copy link
Member

@overvenus overvenus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@NingLin-P
Copy link
Member

/run-unit-test

Copy link
Member

@NingLin-P NingLin-P left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@overvenus overvenus added the status/can-merge Status: Can merge to base branch label Dec 9, 2019
@sre-bot
Copy link
Contributor

sre-bot commented Dec 9, 2019

/run-all-tests

@sre-bot sre-bot merged commit 96c3f97 into tikv:master Dec 9, 2019
@sre-bot
Copy link
Contributor

sre-bot commented Dec 9, 2019

cherry pick to release-3.1 failed

3pointer added a commit to 3pointer/tikv that referenced this pull request Dec 9, 2019
Signed-off-by: luancheng <luancheng@pingcap.com>
3pointer added a commit to 3pointer/tikv that referenced this pull request Dec 10, 2019
Signed-off-by: luancheng <luancheng@pingcap.com>
overvenus pushed a commit that referenced this pull request Dec 10, 2019
* backup: put startKey into backup_file_name to make it unique (#6071)

Signed-off-by: luancheng <luancheng@pingcap.com>

* backup: hash backup file name (#6177)

Signed-off-by: luancheng <luancheng@pingcap.com>
zhang555 pushed a commit to zhang555/tikv that referenced this pull request Dec 16, 2019
Signed-off-by: luancheng <luancheng@pingcap.com>
c1ay pushed a commit to c1ay/tikv that referenced this pull request May 9, 2020
Signed-off-by: luancheng <luancheng@pingcap.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component/backup-restore Component: backup, import, external_storage needs-cherry-pick-release-3.1 Type: Need cherry pick to release 3.1 status/can-merge Status: Can merge to base branch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants