Native support for incremental restore #13239

mszeszko-meta · 2024-12-20T09:40:01Z

Summary

With this change we are adding native library support for incremental restores. When designing the solution we decided to follow 'tiered' approach where users can pick one of the three predefined, and for now, mutually exclusive restore modes (kKeepLatestDbSessionIdFiles, kVerifyChecksum and kPurgeAllFiles [default]) - trading write IO / CPU for the degree of certainty that the existing destination db files match selected backup files contents. New mode option is exposed via existing RestoreOptions configuration, which by this time has been already well-baked into our APIs. Restore engine will consume this configuration and infer which of the existing destination db files are 'in policy' to be retained during restore.

Motivation

This work is motivated by internal customer who is running write-heavy, 1M+ QPS service and is using RocksDB restore functionality to scale up their fleet. Given already high QPS on their end, additional write IO from restores as-is today is contributing to prolonged spikes which lead the service to hit BLOB storage write quotas, which finally results in slowing down the pace of their scaling. See T206217267 for more.

Impact

Enable faster service scaling by reducing write IO footprint on BLOB storage (coming from restore) to the absolute minimum.

Key technical nuances

According to prior investigations, the risk of collisions on [file #, db session id, file size] metadata triplets is low enough to the point that we can confidently use it to uniquely describe the file and its' perceived contents, which is the rationale behind the kKeepLatestDbSessionIdFiles mode. To find more about the risks / tradeoffs for using this mode, please check the related comment in backup_engine.cc. This mode is only supported for SSTs where we persist the db_session_id information in the metadata footer.
kVerifyChecksum mode requires a full blob / SST file scan (assuming backup file has its' checksum_hex metadata set appropriately, if not additional file scan for backup file). While it saves us on write IOs (if checksums match), it's still fairly complex and potentially CPU intensive operation.
We're extending the WorkItemType enum introduced in Generalize work item definition in BackupEngineImpl #13228 to accommodate a new simple request to ComputeChecksum, which will enable us to run 2) in parallel. This will become increasingly more important as we're moving towards disaggregated storage and holding up the sequence of checksum evaluations on a single lagging remote file scan would not be acceptable.
Note that it's necessary to compute the checksum on the restored file if corresponding backup file and existing destination db file checksums didn't match.

Test plan

Manual testing using debugger: ✅
Automated tests:

./backup_engine_test --gtest_filter=*IncrementalRestore* covering the following scenarios: ✅
- Full clean restore
- User workflow simulation: happy path with mix of added new files and deleted original backup files,
- Existing db files corruptions and the difference in handling between kVerifyChecksum and kKeepLatestDbSessionIdFiles modes.
./backup_engine_test --gtest_filter=*ExcludedFiles*
- Integrate existing test collateral with newly introduced restore modes ✅
- Test edge case scenario with excluded file missing across all supplied backups but being present and up to date in db file system. Expectation: Able to restore in kKeepLatestDbSessionIdFiles mode, unable to restore in every other mode. 👷

facebook-github-bot · 2024-12-20T09:42:25Z

@mszeszko-meta has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

facebook-github-bot · 2024-12-20T09:43:22Z

@mszeszko-meta has updated the pull request. You must reimport the pull request before landing.

facebook-github-bot · 2024-12-20T09:44:34Z

@mszeszko-meta has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

facebook-github-bot · 2024-12-20T10:03:01Z

@mszeszko-meta has updated the pull request. You must reimport the pull request before landing.

facebook-github-bot · 2024-12-20T10:06:53Z

@mszeszko-meta has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

facebook-github-bot · 2025-01-03T23:13:47Z

@mszeszko-meta has updated the pull request. You must reimport the pull request before landing.

facebook-github-bot · 2025-01-03T23:38:59Z

@mszeszko-meta has updated the pull request. You must reimport the pull request before landing.

facebook-github-bot · 2025-01-04T00:16:34Z

@mszeszko-meta has updated the pull request. You must reimport the pull request before landing.

facebook-github-bot · 2025-01-06T17:14:01Z

@mszeszko-meta has updated the pull request. You must reimport the pull request before landing.

facebook-github-bot · 2025-01-06T17:50:51Z

@mszeszko-meta has updated the pull request. You must reimport the pull request before landing.

facebook-github-bot · 2025-01-06T17:51:32Z

@mszeszko-meta has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

facebook-github-bot · 2025-01-06T18:58:37Z

@mszeszko-meta has updated the pull request. You must reimport the pull request before landing.

facebook-github-bot · 2025-01-06T18:59:52Z

@mszeszko-meta has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

pdillinger

Looking good, except for compatibility with the obscure "excluded files" feature. I have a bit more to review, but sending this feedback ASAP.

include/rocksdb/utilities/backup_engine.h

utilities/backup/backup_engine.cc

pdillinger · 2025-01-13T23:30:40Z

utilities/backup/backup_engine_test.cc

+  options_.disable_auto_compactions = true;
+  options_.level0_file_num_compaction_trigger = 1000;
+
+  std::vector<std::string> always_copyable_files = {


Explicit list of files like this is fragile to changes in DB operations that might change the order or when we allocate new file numbers. You might have taken inspiration from existing testings in this same test file, but those are subtly either (a) constructing a dummy DB from a set of file names and looking at how they are handled, or (b) injecting extra files into the backup dir to be cleaned up. Do you think we can avoid this?

facebook-github-bot · 2025-01-14T00:27:49Z

@mszeszko-meta has updated the pull request. You must reimport the pull request before landing.

include/rocksdb/utilities/backup_engine.h

facebook-github-bot · 2025-01-14T22:22:39Z

@mszeszko-meta has updated the pull request. You must reimport the pull request before landing.

Summary: As follow-up to facebook#13239, this change is primarily motivated by simplifying the calling conventions of LogAndApply. Since it must be called while holding the DB mutex, it can read safely read cfd->GetLatestMutableCFOptions(), until it releases the mutex. Before it releases the mutex, it makes a copy of the mutable options in a new, unpublished Version object, which can be used when not holding the DB mutex. This eliminates the need for callers of LogAndApply to copy mutable options for its sake, or even specify mutable options at all. And it eliminates the need for *another* copy saved in ManifestWriter. Other functions that don't need the mutable options parameter: * ColumnFamilyData::CreateNewMemtable() * CompactionJob::Install() / InstallCompactionResults() * MemTableList::*InstallMemtable*() * Version::PrepareAppend() Test Plan: existing tests, CI with sanitizers

…llateral.

…files obsure feature

facebook-github-bot · 2025-01-16T04:59:51Z

@mszeszko-meta has updated the pull request. You must reimport the pull request before landing.

facebook-github-bot · 2025-01-16T05:10:29Z

@mszeszko-meta has updated the pull request. You must reimport the pull request before landing.

facebook-github-bot added the CLA Signed label Dec 20, 2024

mszeszko-meta force-pushed the incremental_restore branch from b9cc02d to f9d0de2 Compare December 20, 2024 09:43

mszeszko-meta force-pushed the incremental_restore branch from f9d0de2 to aab8ef8 Compare December 20, 2024 10:02

mszeszko-meta force-pushed the incremental_restore branch from aab8ef8 to 6d5c8e5 Compare January 3, 2025 23:13

mszeszko-meta force-pushed the incremental_restore branch from b665268 to 7f5348f Compare January 4, 2025 00:16

mszeszko-meta force-pushed the incremental_restore branch from bf5dcf5 to 7b3f446 Compare January 6, 2025 17:50

mszeszko-meta requested a review from pdillinger January 10, 2025 23:20

pdillinger requested changes Jan 13, 2025

View reviewed changes

pdillinger reviewed Jan 13, 2025

View reviewed changes

mszeszko-meta force-pushed the incremental_restore branch from c41b139 to 0fb2333 Compare January 14, 2025 00:27

mszeszko-meta commented Jan 14, 2025

View reviewed changes

include/rocksdb/utilities/backup_engine.h Outdated Show resolved Hide resolved

pdillinger mentioned this pull request Jan 15, 2025

Reduce unnecessary MutableCFOptions copies and parameters #13301

Open

Native library support for incremental restore

6957d6c

mszeszko-meta added 6 commits January 15, 2025 20:53

Satisfy linter

1923906

Fix nits

212eda5

Account for excluded files feature in the inference logic

0621722

Introduce restore mode dimension to existing 'excluded files' test co…

efb54d3

…llateral.

Integration with 'external files' (now for real)

c38570a

Update restore mode comment to reflect the integration with external …

69e7299

…files obsure feature

mszeszko-meta force-pushed the incremental_restore branch from 5cfe5e6 to 69e7299 Compare January 16, 2025 04:59

Cleanup

5cadf67

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Native support for incremental restore #13239

Native support for incremental restore #13239

mszeszko-meta commented Dec 20, 2024 •

edited

Loading

facebook-github-bot commented Dec 20, 2024

facebook-github-bot commented Dec 20, 2024

facebook-github-bot commented Dec 20, 2024

facebook-github-bot commented Dec 20, 2024

facebook-github-bot commented Dec 20, 2024

facebook-github-bot commented Jan 3, 2025

facebook-github-bot commented Jan 3, 2025

facebook-github-bot commented Jan 4, 2025

facebook-github-bot commented Jan 6, 2025

facebook-github-bot commented Jan 6, 2025

facebook-github-bot commented Jan 6, 2025

facebook-github-bot commented Jan 6, 2025

facebook-github-bot commented Jan 6, 2025

pdillinger left a comment

pdillinger Jan 13, 2025

facebook-github-bot commented Jan 14, 2025

facebook-github-bot commented Jan 14, 2025

facebook-github-bot commented Jan 16, 2025

facebook-github-bot commented Jan 16, 2025

Native support for incremental restore #13239

Are you sure you want to change the base?

Native support for incremental restore #13239

Conversation

mszeszko-meta commented Dec 20, 2024 • edited Loading

Summary

Motivation

Impact

Key technical nuances

Test plan

facebook-github-bot commented Dec 20, 2024

facebook-github-bot commented Dec 20, 2024

facebook-github-bot commented Dec 20, 2024

facebook-github-bot commented Dec 20, 2024

facebook-github-bot commented Dec 20, 2024

facebook-github-bot commented Jan 3, 2025

facebook-github-bot commented Jan 3, 2025

facebook-github-bot commented Jan 4, 2025

facebook-github-bot commented Jan 6, 2025

facebook-github-bot commented Jan 6, 2025

facebook-github-bot commented Jan 6, 2025

facebook-github-bot commented Jan 6, 2025

facebook-github-bot commented Jan 6, 2025

pdillinger left a comment

Choose a reason for hiding this comment

pdillinger Jan 13, 2025

Choose a reason for hiding this comment

facebook-github-bot commented Jan 14, 2025

facebook-github-bot commented Jan 14, 2025

facebook-github-bot commented Jan 16, 2025

facebook-github-bot commented Jan 16, 2025

mszeszko-meta commented Dec 20, 2024 •

edited

Loading