Complete some of function Implementation in BackupWorker V3#13245
Open
akankshamahajan15 wants to merge 7 commits into
Open
Complete some of function Implementation in BackupWorker V3#13245akankshamahajan15 wants to merge 7 commits into
akankshamahajan15 wants to merge 7 commits into
Conversation
Contributor
Result of foundationdb-pr-clang-ide on Linux RHEL 9
|
Contributor
Result of foundationdb-pr-clang-arm on Linux CentOS 7
|
Contributor
Result of foundationdb-pr-macos-m1 on macOS Ventura 13.x
|
Contributor
Result of foundationdb-pr-clang on Linux RHEL 9
|
Contributor
Result of foundationdb-pr on Linux RHEL 9
|
Contributor
Result of foundationdb-pr-cluster-tests on Linux RHEL 9
|
Contributor
Result of foundationdb-pr-macos on macOS Ventura 13.x
|
b613b7d to
e23bf5a
Compare
Contributor
Result of foundationdb-pr-clang-ide on Linux RHEL 9
|
Contributor
Result of foundationdb-pr-clang-arm on Linux CentOS 7
|
Contributor
Result of foundationdb-pr-cluster-tests on Linux RHEL 9
|
Contributor
Result of foundationdb-pr-macos on macOS Ventura 13.x
|
Contributor
Result of foundationdb-pr-macos-m1 on macOS Ventura 13.x
|
Contributor
Result of foundationdb-pr-clang on Linux RHEL 9
|
Contributor
Result of foundationdb-pr on Linux RHEL 9
|
e23bf5a to
2df0887
Compare
Contributor
Result of foundationdb-pr-clang-ide on Linux RHEL 9
|
Contributor
Result of foundationdb-pr-clang-arm on Linux CentOS 7
|
Contributor
Result of foundationdb-pr on Linux RHEL 9
|
Contributor
Result of foundationdb-pr-clang on Linux RHEL 9
|
Contributor
Result of foundationdb-pr-cluster-tests on Linux RHEL 9
|
Contributor
Result of foundationdb-pr-macos-m1 on macOS Ventura 13.x
|
Contributor
Result of foundationdb-pr-macos on macOS Ventura 13.x
|
2df0887 to
afb46de
Compare
Contributor
Result of foundationdb-pr-clang-ide on Linux RHEL 9
|
Contributor
Result of foundationdb-pr-macos-m1 on macOS Ventura 13.x
|
Contributor
Result of foundationdb-pr-clang on Linux RHEL 9
|
Contributor
Result of foundationdb-pr-clang-arm on Linux CentOS 7
|
Contributor
Result of foundationdb-pr on Linux RHEL 9
|
Contributor
Result of foundationdb-pr-cluster-tests on Linux RHEL 9
|
Contributor
Result of foundationdb-pr-macos on macOS Ventura 13.x
|
Contributor
There was a problem hiding this comment.
Pull request overview
This PR advances BackupWorker V3 (range-partitioned logs) behavior and robustness by restructuring LogSystem initialization, removing V2 readiness-gating keys, and addressing several correctness/cleanup items in the range-partitioned backup worker and client task flow.
Changes:
- Start a dedicated actor to keep
logSystem/oldestBackupEpochupdated fromServerDBInfobefore partition-map processing begins. - Remove use of
BackupConfigreadiness keys (startedBackupWorkers/allWorkerStarted) for RANGE_PARTITIONED_LOG, and skip the corresponding watch inStartFullBackupTaskFunc. - Fix/clean up several range-partitioned worker behaviors (stop/shutdown handling, iterator/loop fixes, redundant cursor calls removal, pop deferral).
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| fdbserver/core/include/fdbserver/core/BackupPartitionMap.h | Modernize type aliases (typedef → using). |
| fdbserver/backupworker/BackupWorkerRangePartitioned.cpp | Core range-partitioned worker correctness/cleanup, LogSystem monitoring actor, shutdown behavior updates. |
| fdbserver/backupworker/BackupWorker.cpp | Minor cleanup (remove unused fields/accessors). |
| fdbclient/FileBackupAgent.cpp | Skip allWorkerStarted watch for RANGE_PARTITIONED_LOG since workers no longer set it. |
afb46de to
b777449
Compare
Contributor
Result of foundationdb-pr-macos on macOS Ventura 13.x
|
Contributor
Result of foundationdb-pr-clang-ide on Linux RHEL 9
|
Contributor
Result of foundationdb-pr-clang-arm on Linux CentOS 7
|
Contributor
Result of foundationdb-pr-clang on Linux RHEL 9
|
Contributor
Result of foundationdb-pr on Linux RHEL 9
|
Contributor
Result of foundationdb-pr-macos-m1 on macOS Ventura 13.x
|
Contributor
Result of foundationdb-pr-cluster-tests on Linux RHEL 9
|
neethuhaneesha
approved these changes
May 30, 2026
|
|
||
| // The task may be restarted. Set the watch if started key has NOT been set. | ||
| if (!taskStarted.get().present()) { | ||
| if (!isRangePartitioned && !taskStarted.get().present()) { |
Contributor
There was a problem hiding this comment.
is it not better if we make it explicit to MutationLogType::PARTITIONED_LOG
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR includes following changes:
Initialize
LogSystembeforePartitionMapis pulled. Pulled thelogSystemupdate logic into a newmonitorLogSystemFromDbInfoactor, started via addActor before the partition-map await.Drop
_updateStartedWorkersand theBackupConfigkeys it managedstartedBackupWorkersandallWorkerStartedexisted for Backup V2 readiness gating. Range-partitioned already gates progress on tagVersions.size() == totalTags, so neither key is needed.Worker no longer reads/writes either; client skips the watch for RANGE_PARTITIONED_LOG.
PARTITIONED_LOG and DEFAULT paths unchanged.
Fix clang-tidy warnings.
Clean up stopped. Removed the dead
per-backup stoppedfield. But keptworker-level stoppedflag + stop() method so uploadData exits cleanly on worker_removed.Simplify and merge the ready functions The pre-existing pattern used static actor helpers behind public wrapper methods, a Flow convention from before C++20 coroutines. Collapsed both _waitReady/waitReady and _waitAllInfoReady/waitAllInfoReady into single member coroutines. Renamed for clarity.
Remove redundant getMore() and nextMessage() in pullPartitionMapFromTLog
Defer pop() for old epochs and on shutdown
Fix iterator double-advance in onBackupChanges
Remove unused variables from BackupWorker.cpp
Testing
Simulation 100k completed:
20260523-013830-ak_bv3-e12f6ac1120c209d compressed=True data_size=37185082 duration=2642536 ended=100000 fail=1 fail_fast=10 max_runs=100000 pass=99999 priority=100 remaining=0 runtime=0:38:25 sanity=False started=100000 stopped=20260523-021655 submitted=20260523-013830 timeout=5400 username=ak_bv3Unrelated failure