-
Notifications
You must be signed in to change notification settings - Fork 1.4k
5s - document and organize knobs #12421
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
Conversation
Result of foundationdb-pr-clang-ide on Linux RHEL 9
|
Result of foundationdb-pr-clang-ide on Linux RHEL 9
|
Result of foundationdb-pr-clang-arm on Linux CentOS 7
|
Result of foundationdb-pr-clang-arm on Linux CentOS 7
|
Result of foundationdb-pr-cluster-tests on Linux RHEL 9
|
Result of foundationdb-pr-cluster-tests on Linux RHEL 9
|
Result of foundationdb-pr on Linux RHEL 9
|
Result of foundationdb-pr-clang on Linux RHEL 9
|
Result of foundationdb-pr-clang on Linux RHEL 9
|
Result of foundationdb-pr on Linux RHEL 9
|
int64_t VERSIONS_PER_SECOND; // Mainly used to represent the rate at which the sequencer can increase the versions | ||
int64_t MAX_READ_TRANSACTION_LIFE_VERSIONS; // Used in various roles (Blob*, DD, LR, RK, SC) but most importantly | ||
// used in CP and SS. Reason for usage is different based on the role. | ||
int64_t MAX_WRITE_TRANSACTION_LIFE_VERSIONS; // Used in Resolver and CP. Reason for usage is different based on the |
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.
applies on the commit path - governs how far back in history a commit request is still accepted. put another way- proxies/resolvers refuse to commit a transaction whose read version is more than this many versions behind the current commit version, returning transaction_too_old
.
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.
updated
|
||
// Versions -- knobs that control 5s timeout | ||
int64_t VERSIONS_PER_SECOND; // Mainly used to represent the rate at which the sequencer can increase the versions | ||
int64_t MAX_READ_TRANSACTION_LIFE_VERSIONS; // Used in various roles (Blob*, DD, LR, RK, SC) but most importantly |
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.
comments can describe these knobs - this one governs how long versioned data stay readable on storage servers. In other words when a client’s read version falls more than this many versions behind the storage servers’ latest durable version, the storage server replies with past_version
. Practically, it’s the MVCC retention budget.
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.
Yeah, I was thinking to add more details but given the variety of usage across roles, I am still figuring out "why" this knob is needed in let's say RK, LR, etc. For SS though, the MVCC reasoning is clear. Your comment makes sense. I'll update the PR.
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.
I believe the LR can never hold more than MAX_READ_TRANSACTION_LIFE_VERSIONS worth of versions in flight (remote storage servers can roll back only that far), but @sbodagala may be more familiar.
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.
when a client’s read version falls more than this many versions behind the storage servers’ latest durable version
I believe instead of durable version, this is the latest committed version (in memory). The actual logic involves min kcv as well:
foundationdb/fdbserver/storageserver.actor.cpp
Lines 12537 to 12538 in 442e150
Version proposedOldestVersion = | |
std::max(data->version.get(), cursor->getMinKnownCommittedVersion()) - maxVersionsInMemory; |
the storage server replies with past_version
The exact error code seems to be transaction_too_old
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.
@dlambrig updated the comment, made some changes based on my understanding above
Result of foundationdb-pr-clang-ide on Linux RHEL 9
|
Result of foundationdb-pr-clang-arm on Linux CentOS 7
|
Result of foundationdb-pr-cluster-tests on Linux RHEL 9
|
Result of foundationdb-pr-clang on Linux RHEL 9
|
Result of foundationdb-pr on Linux RHEL 9
|
Just some light documentation and restructuring before I start making more changes.
100K: 20251006-211028-praza-5s-milestone1-iter30-97f19c55d684ab0de compressed=True data_size=37450858 duration=5307585 ended=100000 fail=1 fail_fast=10 max_runs=100000 pass=99999 priority=100 remaining=0 runtime=1:18:52 sanity=False started=100000 stopped=20251006-222920 submitted=20251006-211028 timeout=5400 username=praza-5s-milestone1-iter30-97f19c55d684ab0dec3b9a56a930d911075cac09. 1 failure in ParallelRestoreOldBackupCorrectnessMultiCycles (TracedTooManyLines), should not be related to this change.
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)