-
Notifications
You must be signed in to change notification settings - Fork 1.4k
Extensions to log server disk queue (made in the context of version vector/unicast) #11777
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
base: version-vector-disk-queue
Are you sure you want to change the base?
Extensions to log server disk queue (made in the context of version vector/unicast) #11777
Conversation
information, needed by the recovery algorithm when unicast is enabled: - PrevVersion of a version - List of log servers that a commit proxy sends a commit version to And, extend the code to populate "unknownCommittedVersions" list (the list of commit versions whose commit status is not known, and to be determined by the recovery version computation algorithm) on a log server restart. NOTE: Please note that these changes will cause version incompatibility and so additional code/logic will need to be added to make sure that upgrades/restart related simulation tests work properly.
committed version onwards in the case where the known committed version is behind LogData::persistentDataDurableVersion (and version vector unicast is enabled). This is so we will populate "LogData::unknownCommittedVersions", on log server restart, with all versions that are needed by unicast recovery.
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-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
|
Result of foundationdb-pr-clang-arm on Linux CentOS 7
|
Result of foundationdb-pr-cluster-tests on Linux CentOS 7
|
fdbserver/TLogServer.actor.cpp
Outdated
logData->unknownCommittedVersions.emplace_front(qe.version, qe.prevVersion, qe.tLogLocIds); | ||
|
||
while (!logData->unknownCommittedVersions.empty() && | ||
logData->unknownCommittedVersions.back().version <= logData->knownCommittedVersion) { | ||
logData->unknownCommittedVersions.pop_back(); | ||
} |
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 you make a function for these lines for code reuse?
There are similar code below and in tLogCommit()
.
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.
Good point. Done now.
…mmittedVersion" onwards in log server's disk queue.
Result of foundationdb-pr-clang-arm on Linux CentOS 7
|
Result of foundationdb-pr-clang on Linux CentOS 7
|
Result of foundationdb-pr-clang-arm on Linux CentOS 7
|
Result of foundationdb-pr on Linux CentOS 7
|
Result of foundationdb-pr-macos on macOS Ventura 13.x
|
Result of foundationdb-pr-macos-m1 on macOS Ventura 13.x
|
Result of foundationdb-pr-cluster-tests on Linux CentOS 7
|
from known committed version onwards, when version vector is enabled.
Result of foundationdb-pr-clang on Linux CentOS 7
|
Result of foundationdb-pr-clang-arm 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 on Linux CentOS 7
|
Result of foundationdb-pr-clang 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-macos on macOS Ventura 13.x
|
Result of foundationdb-pr-cluster-tests 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-macos-m1 on macOS Ventura 13.x
|
Result of foundationdb-pr on Linux CentOS 7
|
Result of foundationdb-pr-macos on macOS Ventura 13.x
|
Result of foundationdb-pr-cluster-tests on Linux CentOS 7
|
150fed7
to
9249ca1
Compare
Result of foundationdb-pr-clang-arm on Linux CentOS 7
|
Result of foundationdb-pr-clang 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-cluster-tests on Linux CentOS 7
|
from, on log server restart, when unicast is enabled.
Result of foundationdb-pr-clang-arm 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-cluster-tests on Linux CentOS 7
|
than the LogData's version in certain cases (on re-start, because the known committed version could get set based on "recoverAt"). So drop the assertion related to this.
Result of foundationdb-pr-clang-arm on Linux CentOS 7
|
Result of foundationdb-pr-clang 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-cluster-tests on Linux CentOS 7
|
Result of foundationdb-pr-macos on macOS Ventura 13.x
|
@@ -1186,7 +1268,9 @@ ACTOR Future<Void> updatePersistentData(TLogData* self, Reference<LogData> logDa | |||
} | |||
if (minVersion != std::numeric_limits<Version>::max()) { | |||
self->persistentQueue->forgetBefore( | |||
newPersistentDataVersion, | |||
(!SERVER_KNOBS->ENABLE_VERSION_VECTOR_TLOG_UNICAST |
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.
In main branch, the first parameter to forgotBefore
on line 1189 is minVersion
, not newPersistentDataVersion
. minVersion
is calculated above. Should minVersion
be used here?
Goal:
The goal is to hold (version incompatible) disk queue related changes, made in the context of getting recovery to work with version vector, in a branch ("version-vector-disk-queue").
Changes:
Commit 37485d2:
And, extend the code to populate "unknownCommittedVersions" list (the list of commit versions whose commit status is not known, and to be determined by the recovery version computation algorithm) on a log server restart.
Commit 0b7fbae:
Extend the log server restart logic to read versions from disk queue from known committed version onwards (instead of from the latest known persistent data durable version). The unicast recovery algorithm needs to know all commit versions, from known committed version onwards, that have been received by a log server in order to correctly compute the recovery version.
Commit c6f99e2:
Do not let a log server delete any versions from known committed version onwards from its disk queue.
Commit dfb2d97:
Make log servers preserve the disk queue positions of all versions from known committed version onwards (when version vector is enabled).
Commit 9249ca1:
This change is related to the change made in PR #11815 in "main".
In case of spill by reference, log servers should use the logic that is based over "TagData::popped" to decide how long to keep the disk queue positions of versions in memory (instead of using the logic that is based over "LogData::persistentDataVersion", which is applicable to spill by value case).
Commit 5837fd1:
Consider all LogData structures while deciding where to start reading the disk queue
from on log server restart.
Commit 16b3aa9:
The known committed version of a LogData structure could be greater than the LogData's version in certain cases (on re-start, because the known committed version could get set based on "revert"). So drop the assertion related to this case.
NOTE: Please note that these changes will cause version incompatibility and so additional code/logic will need to be added to make sure that upgrades/restart related simulation tests work properly.
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)