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

Verify restore from backup in db_stress #4655

Closed
wants to merge 3 commits into from

Conversation

ajkr
Copy link
Contributor

@ajkr ajkr commented Nov 8, 2018

We already exercised backup functionality in db_stress according to the -backup_one_in flag. This PR verifies the backup can be restored/opened and sanity checks a few keys. Changes in this PR:

  • Extracted existing backup-related logic to a helper function, TestBackupRestore
  • Added restore logic, which targets a hidden directory named "./.restore<thread number>", similar to how backups target hidden directories named "./.backup<thread number>".
  • After restore, check the existence/non-existence of a few keys.
  • With this PR, backup is no longer compatible with clearing column families.
  • Also included unrelated fixes to set ReadOptions::total_order_seek=true when using -compare_full_db_state_snapshot

Test Plan:

  • well, it is a test, but I ran it several times and simulated failures to make sure it catches them

Copy link
Contributor

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

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

@ajkr has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

Copy link
Contributor

@riversand963 riversand963 left a comment

Choose a reason for hiding this comment

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

LGTM with a few comments. Thanks @ajkr for improving stress test.

tools/db_stress.cc Outdated Show resolved Hide resolved
tools/db_stress.cc Show resolved Hide resolved
tools/db_stress.cc Show resolved Hide resolved
tools/db_stress.cc Outdated Show resolved Hide resolved
@riversand963
Copy link
Contributor

There are also build failures

Copy link
Contributor Author

@ajkr ajkr left a comment

Choose a reason for hiding this comment

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

thanks for the fast/thorough review!

tools/db_stress.cc Outdated Show resolved Hide resolved
tools/db_stress.cc Outdated Show resolved Hide resolved
tools/db_stress.cc Show resolved Hide resolved
tools/db_stress.cc Show resolved Hide resolved
@facebook-github-bot
Copy link
Contributor

@ajkr has updated the pull request. Re-import the pull request

Copy link
Contributor

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

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

@ajkr has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

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

Successfully merging this pull request may close these issues.

None yet

3 participants