-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
BulkLoad Job and Co-Run BulkLoad and BulkDump #11828
base: main
Are you sure you want to change the base?
Conversation
Result of foundationdb-pr-clang-ide on Linux CentOS 7
|
Result of foundationdb-pr-macos-m1 on macOS Ventura 13.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
|
Result of foundationdb-pr-clang-arm on Linux CentOS 7
|
Result of foundationdb-pr-macos on macOS Ventura 13.x
|
c208163
to
295f750
Compare
Result of foundationdb-pr-clang-ide on Linux CentOS 7
|
Result of foundationdb-pr-macos-m1 on macOS Ventura 13.x
|
Result of foundationdb-pr on Linux CentOS 7
|
Result of foundationdb-pr-clang-arm on Linux CentOS 7
|
Result of foundationdb-pr-clang on Linux CentOS 7
|
Result of foundationdb-pr-cluster-tests on Linux CentOS 7
|
Result of foundationdb-pr-macos on macOS Ventura 13.x
|
@@ -108,7 +108,7 @@ ACTOR Future<UID> bulkDumpCommandActor(Reference<IClusterConnectionRecord> clust | |||
} | |||
std::string remoteRoot = tokens[4].toString(); | |||
KeyRange range = Standalone(KeyRangeRef(rangeBegin, rangeEnd)); | |||
state BulkDumpState bulkDumpJob = newBulkDumpTaskLocalSST(range, remoteRoot); | |||
state BulkDumpState bulkDumpJob = newBulkDumpJobLocalSST(range, remoteRoot); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is good change to do.
fdbclient/BulkDumping.cpp
Outdated
std::vector<uint8_t> byteList; | ||
ASSERT((rawString.size() + 1) % 3 == 0); | ||
for (size_t i = 0; i < rawString.size(); i += 3) { | ||
std::string byteString = rawString.substr(i, 2); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's format of hex here? Characters look like xff or something? If so, is this subset right? Should it be i+1?
fdbclient/BulkDumping.cpp
Outdated
return BulkDumpState( | ||
range, BulkDumpFileType::SST, BulkDumpTransportMethod::CP, BulkDumpExportMethod::File, remoteRoot); | ||
} | ||
|
||
BulkDumpRestoreState newBulkDumpRestoreJobLocalSST(const UID& jobId, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We talked in person on this. BulkLoad is better than BulkDumpRestore IMO
fdbclient/ManagementAPI.actor.cpp
Outdated
state Transaction tr(cx); | ||
loop { | ||
try { | ||
// At most one job at a time, so looking at the first returned range is sufficient |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can I do a bulkload while a bulkdump is going on or is it exclusively one of these operations only?
fdbclient/ManagementAPI.actor.cpp
Outdated
try { | ||
bool existAny = wait(isBulkDumpRestoreAlive(&tr)); | ||
if (existAny) { | ||
return Void(); // early exit |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We will return silently w/o submitting the new job? Shouldn't we let user know there is a job already running?
fdbclient/SystemData.cpp
Outdated
@@ -1226,6 +1226,21 @@ BulkDumpState decodeBulkDumpState(const ValueRef& value) { | |||
return bulkDumpState; | |||
} | |||
|
|||
// Bulk dumping retore keys |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
BulkLoad (smile)
@@ -217,6 +218,19 @@ ACTOR Future<Optional<UID>> existAnyBulkDumpTask(Transaction* tr); | |||
// Get total number of completed tasks within the input range | |||
ACTOR Future<size_t> getBulkDumpCompleteTaskCount(Database cx, KeyRange rangeToRead); | |||
|
|||
// Decide if a bulkDumpRestore job alive | |||
ACTOR Future<bool> isBulkDumpRestoreAlive(Transaction* tr, Optional<UID> jobId); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can this be just isBulkLoad rather than isBulkLoadAlive?
Result of foundationdb-pr-clang-ide on Linux CentOS 7
|
Result of foundationdb-pr-macos-m1 on macOS Ventura 13.x
|
Result of foundationdb-pr-clang-arm 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 Ventura 13.x
|
Result of foundationdb-pr on Linux CentOS 7
|
Result of foundationdb-pr-clang-ide on Linux CentOS 7
|
Result of foundationdb-pr-macos-m1 on macOS Ventura 13.x
|
Result of foundationdb-pr-clang-arm on Linux CentOS 7
|
Result of foundationdb-pr-cluster-tests on Linux CentOS 7
|
Result of foundationdb-pr-clang-ide on Linux CentOS 7
|
Result of foundationdb-pr-macos on macOS Ventura 13.x
|
Result of foundationdb-pr-clang on Linux CentOS 7
|
Result of foundationdb-pr on Linux CentOS 7
|
Result of foundationdb-pr-clang-arm 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 on Linux CentOS 7
|
Result of foundationdb-pr-clang-arm on Linux CentOS 7
|
Result of foundationdb-pr-clang-ide on Linux CentOS 7
|
Result of foundationdb-pr-macos-m1 on macOS Ventura 13.x
|
Result of foundationdb-pr-cluster-tests on Linux CentOS 7
|
Result of foundationdb-pr-macos on macOS Ventura 13.x
|
Result of foundationdb-pr-clang on Linux CentOS 7
|
Result of foundationdb-pr on Linux CentOS 7
|
Result of foundationdb-pr-clang-arm on Linux CentOS 7
|
Result of foundationdb-pr-clang-ide on Linux CentOS 7
|
Result of foundationdb-pr-macos-m1 on macOS Ventura 13.x
|
Result of foundationdb-pr-macos on macOS Ventura 13.x
|
Result of foundationdb-pr-cluster-tests on Linux CentOS 7
|
Result of foundationdb-pr-clang on Linux CentOS 7
|
Result of foundationdb-pr on Linux CentOS 7
|
Replace this text with your description here...
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-branch
ormain
if this is the youngest branch)