Follow-up to PR #12464 by spraza · Pull Request #12541 · apple/foundationdb

spraza · 2025-11-06T19:55:57Z

Description

This PR addresses all remaining feedback in #12464.

Changes:

Verified that between flow and client knobs, no knobs share the same name. But still changed the ClientKnobCollection override behavior in case we add same name knobs in the future, and in general to be consistent with ServerKnobCollection behavior.
Server knob FASTRESTORE_ATOMICOP_WEIGHT is unused in the code, so delete that
Update server VERSIONS_PER_SECOND to use the same value as client version, which is 1M, asme as before.

Testing

Ran 200K:

20251106-075619-praza-pr12464-followup-d27a-9ebc3be04ad2cab4 compressed=True data_size=37437690 duration=5221703 ended=96151 fail=10 fail_fast=10 max_runs=200000 pass=96141 priority=100 remaining=0 runtime=0:52:10 sanity=False started=100113 stopped=20251106-084829 submitted=20251106-075619 timeout=5400 username=praza-pr12464-followup-d27a24c02b743b525b73d329616cac5ec1198a18

Because of some joshua infra issue, this failed early. I can not see any failures from j tail output.

Running two 100Ks again (in progress):

     20251106-195153-praza-pr12464-followup-d27a-9ebc3be04ad2cab4 compressed=True data_size=37437690 duration=5140825 ended=100000 fail_fast=10 max_runs=100000 pass=100000 priority=100 remaining=0 runtime=2:07:59 sanity=False started=100000 stopped=20251106-215952 submitted=20251106-195153 timeout=5400 username=praza-pr12464-followup-d27a24c02b743b525b73d329616cac5ec1198a18
     
     20251106-195156-praza-pr12464-followup-d27a-9ebc3be04ad2cab4 compressed=True data_size=37437690 duration=5203909 ended=100000 fail=1 fail_fast=10 max_runs=100000 pass=99999 priority=100 remaining=0 runtime=2:07:38 sanity=False started=100000 stopped=20251106-215934 submitted=20251106-195156 timeout=5400 username=praza-pr12464-followup-d27a24c02b743b525b73d329616cac5ec1198a18

The first run was 100K/100K pass. The second run was 99999/100K pass, 1 failure was: tests/fast/RandomUnitTests.toml

Sev40 is:

     47         <UnitTest Severity="40"
     48                   ErrorKind="Unset"
     49                   Time="8.250000"
     50                   DateTime="2025-11-06T20:23:54Z"
     51                   Type="UnitTest"
     52                   Machine="3.4.3.4:1"
     53                   ID="0000000000000000"
     54                   Error="end_of_stream"
     55                   ErrorDescription="End of stream"
     56                   ErrorCode="1"
     57                   Name="/flow/thread/ThreadReturnPromiseStream_Seq"
     58                   File="/root/cnd_build_output/fdbrpc/FlowTests.actor.g.cpp"
     59                   Line="14560"
     60                   WallTime="0.514018"
     61                   FlowTime="0.213376"
     62                   ThreadID="6279606179858207310"
     63                   Backtrace="/usr/local/bin/llvm-addr2line -e /var/joshua/ensembles/20251106-195156-praza-pr12464-followup-d27a-9ebc3be04ad2cab4/bin/fdbserver -p -C -f -i 0x555daaf 0x555dda9 0x5557f94 0x41695ec 0x416     63 9306 0x4169764 0x1dfa239 0x52e0ad2 0x52eebe9 0x52ef5d3 0x1d61af8 0x540bb77 0x540b6a0 0x31e519a 0x7f751cc335d0"
     64                   LogGroup="default"
     65                   Roles="TS" **/>**

This test was recently added and I don't think it has anything to do with this PR. I'll start an internal thread about this.

Running another 100K after latest commit:

20251107-230734-praza-pr12464-followup-0c1e-5fafb2e54be0db24 compressed=True data_size=37437746 duration=5761019 ended=100000 fail=1 fail_fast=10 max_runs=100000 pass=99999 priority=100 remaining=0 runtime=0:56:43 sanity=False started=100000 stopped=20251108-000417 submitted=20251107-230734 timeout=5400 username=praza-pr12464-followup-0c1e6053e35cf6aea0b5424f01692aa7aba1468b

It is a non-functional change but will wait for pass before merging. Update: the failure does not look related to this PR.

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.

The PR has a description, explaining both the problem and the solution.
The description mentions which forms of testing were done and the testing seems reasonable.
Every function/class/actor that was touched is reasonably well documented.

For Release-Branches

If this PR is made against a release-branch, please also check the following:

This change/bugfix is a cherry-pick from the next younger branch (younger release-branch or main if this is the youngest branch)
There is a good reason why this PR needs to go into a release branch and this reason is documented (either in the description above or in a linked GitHub issue)

foundationdb-ci · 2025-11-06T20:20:36Z

Result of foundationdb-pr-clang-ide on Linux RHEL 9

Commit ID: d27a24c
Duration 0:24:28
Result: ✅ SUCCEEDED
Error: N/A
Build Log terminal output (available for 30 days)
Build Workspace zip file of the working directory (available for 30 days)

foundationdb-ci · 2025-11-06T20:32:18Z

Result of foundationdb-pr-macos-m1 on macOS Ventura 13.x

Commit ID: d27a24c
Duration 0:36:06
Result: ✅ SUCCEEDED
Error: N/A
Build Log terminal output (available for 30 days)
Build Workspace zip file of the working directory (available for 30 days)

foundationdb-ci · 2025-11-06T20:36:29Z

Result of foundationdb-pr-clang on Linux RHEL 9

Commit ID: d27a24c
Duration 0:40:18
Result: ❌ FAILED
Error: Error while executing command: ctest -j ${NPROC} --no-compress-output -T test --output-on-failure. Reason: exit status 8
Build Log terminal output (available for 30 days)
Build Workspace zip file of the working directory (available for 30 days)

foundationdb-ci · 2025-11-06T20:41:00Z

Result of foundationdb-pr-clang-arm on Linux CentOS 7

Commit ID: d27a24c
Duration 0:44:49
Result: ✅ SUCCEEDED
Error: N/A
Build Log terminal output (available for 30 days)
Build Workspace zip file of the working directory (available for 30 days)

foundationdb-ci · 2025-11-06T20:49:43Z

Result of foundationdb-pr-macos on macOS Ventura 13.x

Commit ID: d27a24c
Duration 0:53:32
Result: ✅ SUCCEEDED
Error: N/A
Build Log terminal output (available for 30 days)
Build Workspace zip file of the working directory (available for 30 days)

foundationdb-ci · 2025-11-06T21:08:34Z

Result of foundationdb-pr-cluster-tests on Linux RHEL 9

Commit ID: d27a24c
Duration 1:12:25
Result: ✅ SUCCEEDED
Error: N/A
Build Log terminal output (available for 30 days)
Build Workspace zip file of the working directory (available for 30 days)
Cluster Test Logs zip file of the test logs (available for 30 days)

foundationdb-ci · 2025-11-06T21:38:35Z

Result of foundationdb-pr on Linux RHEL 9

Commit ID: d27a24c
Duration 1:42:25
Result: ❌ FAILED
Error: Error while executing command: if python3 -m joshua.joshua list --stopped | grep ${ENSEMBLE_ID} | grep -q 'pass=10[0-9][0-9][0-9]'; then echo PASS; else echo FAIL && exit 1; fi. Reason: exit status 1
Build Log terminal output (available for 30 days)
Build Workspace zip file of the working directory (available for 30 days)

foundationdb-ci · 2025-11-06T22:29:01Z

Result of foundationdb-pr-clang-ide on Linux RHEL 9

Commit ID: d27a24c
Duration 0:25:13
Result: ✅ SUCCEEDED
Error: N/A
Build Log terminal output (available for 30 days)
Build Workspace zip file of the working directory (available for 30 days)

foundationdb-ci · 2025-11-06T22:39:20Z

Result of foundationdb-pr-macos-m1 on macOS Ventura 13.x

Commit ID: d27a24c
Duration 0:35:33
Result: ✅ SUCCEEDED
Error: N/A
Build Log terminal output (available for 30 days)
Build Workspace zip file of the working directory (available for 30 days)

foundationdb-ci · 2025-11-06T22:49:24Z

Result of foundationdb-pr-clang-arm on Linux CentOS 7

Commit ID: d27a24c
Duration 0:45:35
Result: ✅ SUCCEEDED
Error: N/A
Build Log terminal output (available for 30 days)
Build Workspace zip file of the working directory (available for 30 days)

foundationdb-ci · 2025-11-06T22:56:58Z

Result of foundationdb-pr-macos on macOS Ventura 13.x

Commit ID: d27a24c
Duration 0:53:12
Result: ✅ SUCCEEDED
Error: N/A
Build Log terminal output (available for 30 days)
Build Workspace zip file of the working directory (available for 30 days)

foundationdb-ci · 2025-11-06T23:10:30Z

Result of foundationdb-pr on Linux RHEL 9

Commit ID: d27a24c
Duration 1:06:42
Result: ✅ SUCCEEDED
Error: N/A
Build Log terminal output (available for 30 days)
Build Workspace zip file of the working directory (available for 30 days)

foundationdb-ci · 2025-11-06T23:14:14Z

Result of foundationdb-pr-cluster-tests on Linux RHEL 9

Commit ID: d27a24c
Duration 1:10:25
Result: ✅ SUCCEEDED
Error: N/A
Build Log terminal output (available for 30 days)
Build Workspace zip file of the working directory (available for 30 days)
Cluster Test Logs zip file of the test logs (available for 30 days)

foundationdb-ci · 2025-11-06T23:14:41Z

Result of foundationdb-pr-clang on Linux RHEL 9

Commit ID: d27a24c
Duration 1:10:56
Result: ✅ SUCCEEDED
Error: N/A
Build Log terminal output (available for 30 days)
Build Workspace zip file of the working directory (available for 30 days)

sbodagala · 2025-11-07T18:42:00Z


 	// Versions -- knobs that control 5s timeout
-	init( VERSIONS_PER_SECOND,                                   1e6 );
+	init( VERSIONS_PER_SECOND,            clientKnobs->VERSIONS_PER_SECOND );


I think you misunderstood my question. It was not about this knob (VERSIONS_PER_SECOND) but the knob below it (which you modified in PR #12464). I was trying to ask this: you set that server knob's value to the corresponding client knob's value in commit b2633d2 in that PR, but in commit 906789b (in that PR) you modified that to be assigned randomly. Is that an intentional change?

Please revert this change to VERSIONS_PER_SECOND. Thanks!

I see, you seem to be referring to MAX_READ_TRANSACTION_LIFE_VERSIONS. That is not present in client knobs. Client only uses MAX_WRITE_TRANSACTION_LIFE_VERSIONS.

I will revert this change then.

Relatedly, I wonder why github did not show the right line. I see this:

So R35 seems correct and is pointing to VERSIONS_PER_SECOND. Maybe you put in a comment on an older version of the PR and github got confused.

you set that server knob's value to the corresponding client knob's value in commit b2633d2 in that PR, but in commit 906789b (in that PR) you modified that to be assigned randomly.

b2633d2 changes ServerKnobs::MAX_WRITE_TRANSACTION_LIFE_VERSIONS to use clientKnobs::MAX_WRITE_TRANSACTION_LIFE_VERSIONS.

906789b changes MAX_READ_TRANSACTION_LIFE_VERSIONS to just use the new function name (getSimulatedTxnTimeoutSeconds instead of randomTxnTimeoutSeconds).

I caused the confusion, apologies! I follow your changes now, thanks!

foundationdb-ci · 2025-11-07T23:24:14Z

Result of foundationdb-pr-clang-ide on Linux RHEL 9

Commit ID: 0c1e605
Duration 0:24:08
Result: ✅ SUCCEEDED
Error: N/A
Build Log terminal output (available for 30 days)
Build Workspace zip file of the working directory (available for 30 days)

foundationdb-ci · 2025-11-07T23:35:56Z

Result of foundationdb-pr-macos-m1 on macOS Ventura 13.x

Commit ID: 0c1e605
Duration 0:35:52
Result: ✅ SUCCEEDED
Error: N/A
Build Log terminal output (available for 30 days)
Build Workspace zip file of the working directory (available for 30 days)

foundationdb-ci · 2025-11-07T23:47:14Z

Result of foundationdb-pr-clang-arm on Linux CentOS 7

Commit ID: 0c1e605
Duration 0:47:10
Result: ✅ SUCCEEDED
Error: N/A
Build Log terminal output (available for 30 days)
Build Workspace zip file of the working directory (available for 30 days)

foundationdb-ci · 2025-11-07T23:52:49Z

Result of foundationdb-pr-macos on macOS Ventura 13.x

Commit ID: 0c1e605
Duration 0:52:46
Result: ✅ SUCCEEDED
Error: N/A
Build Log terminal output (available for 30 days)
Build Workspace zip file of the working directory (available for 30 days)

foundationdb-ci · 2025-11-08T00:04:24Z

Result of foundationdb-pr-clang on Linux RHEL 9

Commit ID: 0c1e605
Duration 1:04:18
Result: ✅ SUCCEEDED
Error: N/A
Build Log terminal output (available for 30 days)
Build Workspace zip file of the working directory (available for 30 days)

foundationdb-ci · 2025-11-08T00:05:58Z

Result of foundationdb-pr on Linux RHEL 9

Commit ID: 0c1e605
Duration 1:05:55
Result: ❌ FAILED
Error: Error while executing command: if python3 -m joshua.joshua list --stopped | grep ${ENSEMBLE_ID} | grep -q 'pass=10[0-9][0-9][0-9]'; then echo PASS; else echo FAIL && exit 1; fi. Reason: exit status 1
Build Log terminal output (available for 30 days)
Build Workspace zip file of the working directory (available for 30 days)

foundationdb-ci · 2025-11-08T00:08:38Z

Result of foundationdb-pr-cluster-tests on Linux RHEL 9

Commit ID: 0c1e605
Duration 1:08:35
Result: ✅ SUCCEEDED
Error: N/A
Build Log terminal output (available for 30 days)
Build Workspace zip file of the working directory (available for 30 days)
Cluster Test Logs zip file of the test logs (available for 30 days)

foundationdb-ci · 2025-11-08T00:37:18Z

Result of foundationdb-pr-clang-ide on Linux RHEL 9

Commit ID: 0c1e605
Duration 0:23:57
Result: ✅ SUCCEEDED
Error: N/A
Build Log terminal output (available for 30 days)
Build Workspace zip file of the working directory (available for 30 days)

foundationdb-ci · 2025-11-08T00:48:54Z

Result of foundationdb-pr-macos-m1 on macOS Ventura 13.x

Commit ID: 0c1e605
Duration 0:35:28
Result: ✅ SUCCEEDED
Error: N/A
Build Log terminal output (available for 30 days)
Build Workspace zip file of the working directory (available for 30 days)

foundationdb-ci · 2025-11-08T01:01:17Z

Result of foundationdb-pr-clang-arm on Linux CentOS 7

Commit ID: 0c1e605
Duration 0:47:54
Result: ✅ SUCCEEDED
Error: N/A
Build Log terminal output (available for 30 days)
Build Workspace zip file of the working directory (available for 30 days)

foundationdb-ci · 2025-11-08T01:05:11Z

Result of foundationdb-pr-clang on Linux RHEL 9

Commit ID: 0c1e605
Duration 0:51:49
Result: ✅ SUCCEEDED
Error: N/A
Build Log terminal output (available for 30 days)
Build Workspace zip file of the working directory (available for 30 days)

foundationdb-ci · 2025-11-08T01:06:06Z

Result of foundationdb-pr-macos on macOS Ventura 13.x

Commit ID: 0c1e605
Duration 0:52:44
Result: ✅ SUCCEEDED
Error: N/A
Build Log terminal output (available for 30 days)
Build Workspace zip file of the working directory (available for 30 days)

foundationdb-ci · 2025-11-08T01:13:37Z

Result of foundationdb-pr on Linux RHEL 9

Commit ID: 0c1e605
Duration 1:00:15
Result: ❌ FAILED
Error: Error while executing command: if python3 -m joshua.joshua list --stopped | grep ${ENSEMBLE_ID} | grep -q 'pass=10[0-9][0-9][0-9]'; then echo PASS; else echo FAIL && exit 1; fi. Reason: exit status 1
Build Log terminal output (available for 30 days)
Build Workspace zip file of the working directory (available for 30 days)

foundationdb-ci · 2025-11-08T01:20:14Z

Result of foundationdb-pr-cluster-tests on Linux RHEL 9

Commit ID: 0c1e605
Duration 1:06:49
Result: ✅ SUCCEEDED
Error: N/A
Build Log terminal output (available for 30 days)
Build Workspace zip file of the working directory (available for 30 days)
Cluster Test Logs zip file of the test logs (available for 30 days)

foundationdb-ci · 2025-11-08T09:20:40Z

Result of foundationdb-pr-clang-ide on Linux RHEL 9

Commit ID: 0c1e605
Duration 0:24:31
Result: ✅ SUCCEEDED
Error: N/A
Build Log terminal output (available for 30 days)
Build Workspace zip file of the working directory (available for 30 days)

foundationdb-ci · 2025-11-08T09:31:43Z

Result of foundationdb-pr-macos-m1 on macOS Ventura 13.x

Commit ID: 0c1e605
Duration 0:35:35
Result: ✅ SUCCEEDED
Error: N/A
Build Log terminal output (available for 30 days)
Build Workspace zip file of the working directory (available for 30 days)

foundationdb-ci · 2025-11-08T09:41:35Z

Result of foundationdb-pr-clang-arm on Linux CentOS 7

Commit ID: 0c1e605
Duration 0:45:28
Result: ✅ SUCCEEDED
Error: N/A
Build Log terminal output (available for 30 days)
Build Workspace zip file of the working directory (available for 30 days)

foundationdb-ci · 2025-11-08T09:48:33Z

Result of foundationdb-pr-macos on macOS Ventura 13.x

Commit ID: 0c1e605
Duration 0:52:26
Result: ✅ SUCCEEDED
Error: N/A
Build Log terminal output (available for 30 days)
Build Workspace zip file of the working directory (available for 30 days)

foundationdb-ci · 2025-11-08T10:00:28Z

Result of foundationdb-pr-clang on Linux RHEL 9

Commit ID: 0c1e605
Duration 1:04:19
Result: ✅ SUCCEEDED
Error: N/A
Build Log terminal output (available for 30 days)
Build Workspace zip file of the working directory (available for 30 days)

foundationdb-ci · 2025-11-08T10:01:59Z

Result of foundationdb-pr on Linux RHEL 9

Commit ID: 0c1e605
Duration 1:05:53
Result: ✅ SUCCEEDED
Error: N/A
Build Log terminal output (available for 30 days)
Build Workspace zip file of the working directory (available for 30 days)

foundationdb-ci · 2025-11-08T10:08:13Z

Result of foundationdb-pr-cluster-tests on Linux RHEL 9

Commit ID: 0c1e605
Duration 1:12:06
Result: ✅ SUCCEEDED
Error: N/A
Build Log terminal output (available for 30 days)
Build Workspace zip file of the working directory (available for 30 days)
Cluster Test Logs zip file of the test logs (available for 30 days)

* remove server FASTRESTORE_ATOMICOP_WEIGHT * address other feedback * fix spacing * revert VERSIONS_PER_SECOND change

spraza added 3 commits November 5, 2025 23:19

remove server FASTRESTORE_ATOMICOP_WEIGHT

5416916

address other feedback

39cf32f

fix spacing

d27a24c

spraza requested a review from sbodagala November 6, 2025 19:56

spraza closed this Nov 6, 2025

spraza reopened this Nov 6, 2025

sbodagala reviewed Nov 7, 2025

View reviewed changes

sbodagala approved these changes Nov 7, 2025

View reviewed changes

revert VERSIONS_PER_SECOND change

0c1e605

spraza closed this Nov 8, 2025

spraza reopened this Nov 8, 2025

spraza closed this Nov 8, 2025

spraza reopened this Nov 8, 2025

spraza merged commit f21ae29 into apple:main Nov 10, 2025
7 checks passed

spraza added a commit to spraza/foundationdb that referenced this pull request Jan 15, 2026

Follow-up to PR apple#12464 (apple#12541)

dae9d37

* remove server FASTRESTORE_ATOMICOP_WEIGHT * address other feedback * fix spacing * revert VERSIONS_PER_SECOND change

spraza added a commit to spraza/foundationdb that referenced this pull request Jan 15, 2026

Follow-up to PR apple#12464 (apple#12541)

5aada41

* remove server FASTRESTORE_ATOMICOP_WEIGHT * address other feedback * fix spacing * revert VERSIONS_PER_SECOND change

Uh oh!

Conversation

spraza commented Nov 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Testing

Code-Reviewer Section

For Release-Branches

Uh oh!

foundationdb-ci commented Nov 6, 2025

Result of foundationdb-pr-clang-ide on Linux RHEL 9

Uh oh!

foundationdb-ci commented Nov 6, 2025

Result of foundationdb-pr-macos-m1 on macOS Ventura 13.x

Uh oh!

foundationdb-ci commented Nov 6, 2025

Result of foundationdb-pr-clang on Linux RHEL 9

Uh oh!

foundationdb-ci commented Nov 6, 2025

Result of foundationdb-pr-clang-arm on Linux CentOS 7

Uh oh!

foundationdb-ci commented Nov 6, 2025

Result of foundationdb-pr-macos on macOS Ventura 13.x

Uh oh!

foundationdb-ci commented Nov 6, 2025

Result of foundationdb-pr-cluster-tests on Linux RHEL 9

Uh oh!

foundationdb-ci commented Nov 6, 2025

Result of foundationdb-pr on Linux RHEL 9

Uh oh!

foundationdb-ci commented Nov 6, 2025

Result of foundationdb-pr-clang-ide on Linux RHEL 9

Uh oh!

foundationdb-ci commented Nov 6, 2025

Result of foundationdb-pr-macos-m1 on macOS Ventura 13.x

Uh oh!

foundationdb-ci commented Nov 6, 2025

Result of foundationdb-pr-clang-arm on Linux CentOS 7

Uh oh!

foundationdb-ci commented Nov 6, 2025

Result of foundationdb-pr-macos on macOS Ventura 13.x

Uh oh!

foundationdb-ci commented Nov 6, 2025

Result of foundationdb-pr on Linux RHEL 9

Uh oh!

foundationdb-ci commented Nov 6, 2025

Result of foundationdb-pr-cluster-tests on Linux RHEL 9

Uh oh!

foundationdb-ci commented Nov 6, 2025

Result of foundationdb-pr-clang on Linux RHEL 9

Uh oh!

sbodagala Nov 7, 2025

Choose a reason for hiding this comment

Uh oh!

sbodagala Nov 7, 2025

Choose a reason for hiding this comment

Uh oh!

spraza Nov 7, 2025

Choose a reason for hiding this comment

Uh oh!

spraza Nov 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sbodagala Nov 7, 2025

Choose a reason for hiding this comment

Uh oh!

foundationdb-ci commented Nov 7, 2025

Result of foundationdb-pr-clang-ide on Linux RHEL 9

Uh oh!

foundationdb-ci commented Nov 7, 2025

Result of foundationdb-pr-macos-m1 on macOS Ventura 13.x

Uh oh!

foundationdb-ci commented Nov 7, 2025

Result of foundationdb-pr-clang-arm on Linux CentOS 7

Uh oh!

foundationdb-ci commented Nov 7, 2025

Result of foundationdb-pr-macos on macOS Ventura 13.x

Uh oh!

foundationdb-ci commented Nov 8, 2025

spraza commented Nov 6, 2025 •

edited

Loading

spraza Nov 7, 2025 •

edited

Loading