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

More backup/restore stress test fixes #7361

Closed
wants to merge 1 commit into from

Conversation

pdillinger
Copy link
Contributor

@pdillinger pdillinger commented Sep 9, 2020

Summary: (a) Missed a case in updating handling of rand_keys
(b) Only opening restored db with DB::Open so don't (yet)
attempt to open restored BlobDB or TransactionDB.

Test Plan: better than being broken

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.

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

Copy link
Contributor

@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.

LGTM!

std::string restore_dir = FLAGS_db + "/.restore" + ToString(thread->tid);
// Reasons for this:
// * Free up disk space ASAP after crash & restart
// * Attempting to restore from backup from prior invocation could fail
Copy link
Contributor

Choose a reason for hiding this comment

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

Just wondering, do you have an example of such an option?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Here's the failure I'm trying to fix:

Failure in DB::Open in backup/restore with: Not implemented: WritePrepared/WriteUnprepared txn tag when write_after_commit_ is enabled (in default WriteCommitted mode). If it is not due to corruption, the WAL must be emptied before changing the WritePolicy.

After private chat, my diagnosis is a bit wrong. We can't (necessarily) use vanilla DB::Open when blobdb or txndb are enabled. Revision coming

Summary: (a) Missed a case in updating handling of rand_keys
(b) Only opening restored db with DB::Open so don't (yet)
attempt to open restored BlobDB or TransactionDB.

Test Plan: better than being broken
@facebook-github-bot
Copy link
Contributor

@pdillinger has updated the pull request. You must reimport the pull request before landing.

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.

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

@facebook-github-bot
Copy link
Contributor

@pdillinger merged this pull request in e314935.

codingrhythm pushed a commit to SafetyCulture/rocksdb that referenced this pull request Mar 5, 2021
Summary:
(a) Missed a case in updating handling of rand_keys
(b) Only opening restored db with DB::Open so don't (yet)
attempt to open restored BlobDB or TransactionDB.

Pull Request resolved: facebook#7361

Test Plan: better than being broken

Reviewed By: ajkr

Differential Revision: D23592570

Pulled By: pdillinger

fbshipit-source-id: dd1d999bcc0c852ee77cb6041964ec4abc0fd4fd
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.

3 participants