Skip to content

Remove parallel restore feature#12107

Closed
jzhou77 wants to merge 2 commits into
apple:mainfrom
jzhou77:remove-parallel-restore
Closed

Remove parallel restore feature#12107
jzhou77 wants to merge 2 commits into
apple:mainfrom
jzhou77:remove-parallel-restore

Conversation

@jzhou77

@jzhou77 jzhou77 commented Apr 25, 2025

Copy link
Copy Markdown
Collaborator

This feature is removed and we'll use bulk loading mechanism for restoring a cluster in the future.

Put this in draft mode until new backup is fully qualified.

20250425-051235-jzhou-7392bc7100f7123f compressed=True data_size=35263669 duration=5404578 ended=100000 fail=2 fail_fast=10 max_runs=100000 pass=99998 priority=100 remaining=0 runtime=0:54:19 sanity=False started=100000 stopped=20250425-060654 submitted=20250425-051235 timeout=5400 username=jzhou

2 failures are unrelated:

-f ./tests/fast/KillRegionCycle.toml -s 3267172411 -b on
-f ./tests/rare/BlobRestoreTenantMode.toml -s 3245633057 -b on

Code-Reviewer Section

The general pull request guidelines can be found here.

Please check each of the following things and check all boxes before accepting a PR.

  • The PR has a description, explaining both the problem and the solution.
  • The description mentions which forms of testing were done and the testing seems reasonable.
  • Every function/class/actor that was touched is reasonably well documented.

For Release-Branches

If this PR is made against a release-branch, please also check the following:

  • This change/bugfix is a cherry-pick from the next younger branch (younger release-branch or main if this is the youngest branch)
  • There is a good reason why this PR needs to go into a release branch and this reason is documented (either in the description above or in a linked GitHub issue)

jzhou77 added 2 commits April 25, 2025 09:18
This feature is removed and we'll use bulk loading mechanism for restoring a
cluster in the future.
20250425-051235-jzhou-7392bc7100f7123f
@jzhou77 jzhou77 closed this Mar 19, 2026
gxglass added a commit that referenced this pull request Apr 1, 2026
This parallel restore feature has been slated for removal for at least a year. This PR is closely based on earlier PR #12107.

This blog post explains some of the problems with the parallel restore feature: https://medium.com/@jingyuzhou/why-foundationdb-restore-is-slow-and-what-can-be-done-about-it-e73a821fdd33

As far as large feature removal changes go, this one is very straightforward, with most relevant files and test cases simply being deleted. There is one knob rename where storageserver.actor.cpp was using a knob with FASTRESTORE in the name. Other than that, changes to shared files mainly involve removing fastrestore-specific CLI and role support.

In progress:
20260330-222511-gglass-5ee0142213471b70 compressed=True data_size=35343375 duration=4611964 ended=100000 fail=1 fail_fast=1000 max_runs=100000 pass=99999 priority=100 remaining=0 runtime=0:58:23 sanity=False started=100000 stopped=20260330-232334 submitted=20260330-222511 timeout=5400 username=gglass

The one failure was in SwizzledCycleTest.toml with too many lines of output and a timeout. I kind of suspect it's unrelated but haven't looked further.
* Remove parallel restore feature.  This is based on earlier PR 12107.  Compiles but untested.

* AI generated commit:

⏺ The fix restores a single if block that was accidentally deleted when removing the FASTRESTORE_TOOL code:

  if (!restoreSystemKeys && !restoreUserKeys && backupKeys.empty()) {
      addDefaultBackupRanges(backupKeys);
  }

  When no explicit key ranges are specified on the command line and neither --user-data nor --system-metadata flags are set, this populates backupKeys with the default backup ranges
  (essentially all user data). Without it, backupKeys stays empty and hits the ASSERT(!backupRanges.empty()) in submitBackup().

* Remove a believed-to-be-dead code path, and update .gitignore

* Remove duplicate definition of restoreRequestDoneKey
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant