Enhance fdbbackup query command to estimate data processing from a specific snapshot to a target version#9485
Conversation
Result of foundationdb-pr-clang-ide on Linux CentOS 7
|
Result of foundationdb-pr-clang on Linux CentOS 7
|
Result of foundationdb-pr on Linux CentOS 7
|
Result of foundationdb-pr-macos on macOS Monterey 12.x
|
Result of foundationdb-pr-macos-m1 on macOS Monterey 12.x
|
Result of foundationdb-pr-cluster-tests on Linux CentOS 7
|
Result of foundationdb-pr-clang-ide on Linux CentOS 7
|
Doxense CI Report for Windows 10
|
Doxense CI Report for Windows 10
|
Result of foundationdb-pr-macos-m1 on macOS Monterey 12.x
|
Result of foundationdb-pr-cluster-tests on Linux CentOS 7
|
Result of foundationdb-pr-macos on macOS Monterey 12.x
|
Result of foundationdb-pr on Linux CentOS 7
|
Result of foundationdb-pr-clang on Linux CentOS 7
|
| if (targetVersion == earliestVersion) { | ||
| i = 0; |
There was a problem hiding this comment.
This doesn't work with the logic below, i.e., loop at line 933. There we do i--, so if 0 is not desirable, we won't try next ones like 1, 2, ... So the change needed is to change the loop to be i++ if earliestVersion is the target version.
There was a problem hiding this comment.
Both way may not work because the snapshot at head or tail may be incomplete.
So I changed the behavior to explicitly selection the restore version before entering this command based on backup description. PTAL.
…on in query command in stead of going throw snapshots
Result of foundationdb-pr-clang-ide on Linux CentOS 7
|
Result of foundationdb-pr-clang-ide on Linux CentOS 7
|
Doxense CI Report for Windows 10
|
Result of foundationdb-pr-macos-m1 on macOS Monterey 12.x
|
Result of foundationdb-pr on Linux CentOS 7
|
Result of foundationdb-pr-cluster-tests on Linux CentOS 7
|
Result of foundationdb-pr-clang on Linux CentOS 7
|
Result of foundationdb-pr-macos on macOS Monterey 12.x
|
Result of foundationdb-pr-clang on Linux CentOS 7
|
Result of foundationdb-pr on Linux CentOS 7
|
Result of foundationdb-pr-cluster-tests on Linux CentOS 7
|
| TraceEvent("BackupQueryFailure").detail("OperationId", operationId).detail("Reason", errorMessage); | ||
| } | ||
|
|
||
| std::pair<Version, Version> getMaxMinRestorableVersions(const BackupDescription& desc, bool mayOnlyApplyMutationLog) { |
There was a problem hiding this comment.
I don't understand why mayOnlyApplyMutationLog is !keyRangesFilter.empty().
There was a problem hiding this comment.
I don't understand that either. The code simply preserves previous behavior. Looks like if backup doesn't have a restorable version, and it has KeyRangeFilter set, it just tries to use the mutation log only.
I don't see this logis is used anywhere else. I can also remove it.
There was a problem hiding this comment.
I'm fine either way. If we keep this logic, maybe add a comment on this.
There was a problem hiding this comment.
I added a comment. This option (hopefully) won't be used in our case.
| wait(store(fileSet, bc->getRestoreSet(restoreVersion, keyRangesFilter))); | ||
| } else { | ||
| // We only need to know all the log files from snapshotVersion to restoreVersion. | ||
| wait(store(fileSet, bc->getRestoreSet(restoreVersion, keyRangesFilter, /*logOnly=*/true, snapshotVersion))); |
There was a problem hiding this comment.
how different is this from 2801? can we consolidate these two calls?
There was a problem hiding this comment.
the 2801 code block is to calculate backup data "up to the user-configured snapshot version".
this code back adds up the backup data from snapshot version to restorable version.
There was a problem hiding this comment.
virtual Future<Optional> getRestoreSet(Version targetVersion,
VectorRef keyRangesFilter = {},
bool logsOnly = false,
Version beginVersion = -1) = 0;
There was a problem hiding this comment.
I restructured the code to make it more readable. PTAL.
| " version approximately NUM_DAYS days worth of versions prior to the latest log version in " | ||
| "the backup.\n"); | ||
| printf(" --query-restore-snapshot-version VERSION\n" | ||
| " For query operations, set snapshot version for restoring a backup. Set -1 for maximum\n" |
There was a problem hiding this comment.
" For query operations, set snapshot version for restoring a backup. " ->
" For query operations, set the snapshot version, inclusive, used to restore a backup. "
for better readability
There was a problem hiding this comment.
" Set -1 for maximumrestorable version (default) and -3 for minimum restorable version."
Change to
"
Set -1 to use the latest valid snapshot,
Set -3 to use the oldest valid snapshot.
"
| wait(store(fileSet, bc->getRestoreSet(restoreVersion, keyRangesFilter))); | ||
| } else { | ||
| // We only need to know all the log files from snapshotVersion to restoreVersion. | ||
| wait(store(fileSet, bc->getRestoreSet(restoreVersion, keyRangesFilter, /*logOnly=*/true, snapshotVersion))); |
There was a problem hiding this comment.
the 2801 code block is to calculate backup data "up to the user-configured snapshot version".
this code back adds up the backup data from snapshot version to restorable version.
| wait(store(fileSet, bc->getRestoreSet(restoreVersion, keyRangesFilter))); | ||
| } else { | ||
| // We only need to know all the log files from snapshotVersion to restoreVersion. | ||
| wait(store(fileSet, bc->getRestoreSet(restoreVersion, keyRangesFilter, /*logOnly=*/true, snapshotVersion))); |
There was a problem hiding this comment.
virtual Future<Optional> getRestoreSet(Version targetVersion,
VectorRef keyRangesFilter = {},
bool logsOnly = false,
Version beginVersion = -1) = 0;
Result of foundationdb-pr-clang-ide on Linux CentOS 7
|
Result of foundationdb-pr-cluster-tests on Linux CentOS 7
|
Result of foundationdb-pr-macos on macOS Monterey 12.x
|
Result of foundationdb-pr on Linux CentOS 7
|
Result of foundationdb-pr-clang on Linux CentOS 7
|
Doxense CI Report for Windows 10
|
earliestVersionconstant.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.
For Release-Branches
If this PR is made against a release-branch, please also check the following:
release-branchormainif this is the youngest branch)