Make log servers return an empty version range only when it is correct to do so #12171

sbodagala · 2025-05-28T22:16:48Z

In the context of version vector/unicast, make log servers return an empty version range (on peeks) such that the receiver won't miss any versions that it is supposed to receive.

Make the cluster controller collect the set of log servers participated in recovery and propagate that information to the other processes
Extend the ServerPeekCursor to take a flag that tells the source log server whether it can return an empty version range or not (in the context of version vector/unicast), and make the source log server return an empty version range only when this flag is set
Make the peek APIs set the flag appropriately when initializing ServerPeekCursors
Also, take the internal logic used by SetPeekCursor and MergedPeekCursor into account while initializing the above mentioned flag.

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)

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.

…mmittedVersion" onwards in log server's disk queue.

from known committed version onwards, when version vector is enabled.

apple#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).

an empty version range (on peeks) such that the receiver won't miss any versions that it is supposed to receive. - Make the cluster controller collect the set of log servers participated in recovery and propagate that information to the other processes - Extend the ServerPeekCursor to take a flag that tells the source log server whether it can return an empty version range or not (in the context of version vector/unicast), and make the source log server return an empty version range only when this flag is set - Make the peek APIs set the flag appropriately when initializing ServerPeekCursors - Also, take the internal logic used by SetPeekCursor and MergedPeekCursor into account while initializing the above mentioned flag.

sbodagala · 2025-05-28T22:18:42Z

NOTE: I didn't do a good job of making this branch. Please note that the very last commit is the only commit that is relevant to this task, thanks!

foundationdb-ci · 2025-05-28T22:21:46Z

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

Commit ID: 9a34167
Duration 0:04:48
Result: ❌ FAILED
Error: Error while executing command: if [[ $(git diff --shortstat 2> /dev/null | tail -n1) == "" ]]; then echo "CODE FORMAT CLEAN"; else echo "CODE FORMAT NOT CLEAN"; echo; echo "THE FOLLOWING FILES NEED TO BE FORMATTED"; echo; git ls-files -m; echo; if [[ $FDB_VERSION =~ 7\.\3. ]]; then echo skip; else exit 1; fi; 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-05-28T22:21:58Z

Result of foundationdb-pr on Linux RHEL 9

Commit ID: 9a34167
Duration 0:05:00
Result: ❌ FAILED
Error: Error while executing command: if [[ $(git diff --shortstat 2> /dev/null | tail -n1) == "" ]]; then echo "CODE FORMAT CLEAN"; else echo "CODE FORMAT NOT CLEAN"; echo; echo "THE FOLLOWING FILES NEED TO BE FORMATTED"; echo; git ls-files -m; echo; if [[ $FDB_VERSION =~ 7\.\3. ]]; then echo skip; else exit 1; fi; 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-05-28T22:22:00Z

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

Commit ID: 9a34167
Duration 0:05:03
Result: ❌ FAILED
Error: Error while executing command: if [[ $(git diff --shortstat 2> /dev/null | tail -n1) == "" ]]; then echo "CODE FORMAT CLEAN"; else echo "CODE FORMAT NOT CLEAN"; echo; echo "THE FOLLOWING FILES NEED TO BE FORMATTED"; echo; git ls-files -m; echo; if [[ $FDB_VERSION =~ 7\.\3. ]]; then echo skip; else exit 1; fi; 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)
Cluster Test Logs zip file of the test logs (available for 30 days)

foundationdb-ci · 2025-05-28T22:22:06Z

Result of foundationdb-pr-clang on Linux RHEL 9

Commit ID: 9a34167
Duration 0:05:07
Result: ❌ FAILED
Error: Error while executing command: if [[ $(git diff --shortstat 2> /dev/null | tail -n1) == "" ]]; then echo "CODE FORMAT CLEAN"; else echo "CODE FORMAT NOT CLEAN"; echo; echo "THE FOLLOWING FILES NEED TO BE FORMATTED"; echo; git ls-files -m; echo; if [[ $FDB_VERSION =~ 7\.\3. ]]; then echo skip; else exit 1; fi; 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-05-28T22:22:08Z

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

Commit ID: 9a34167
Duration 0:05:07
Result: ❌ FAILED
Error: Error while executing command: if [[ $(git diff --shortstat 2> /dev/null | tail -n1) == "" ]]; then echo "CODE FORMAT CLEAN"; else echo "CODE FORMAT NOT CLEAN"; echo; echo "THE FOLLOWING FILES NEED TO BE FORMATTED"; echo; git ls-files -m; echo; if [[ $FDB_VERSION =~ 7\.\3. ]]; then echo skip; else exit 1; fi; 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)

sbodagala · 2025-05-28T22:31:10Z

fdbserver/LogRouter.cpp

@@ -390,8 +393,8 @@ Future<Reference<ILogSystem::IPeekCursor>> LogRouterData::getPeekCursorData(Refe
 		    .When(logSystemChanged,
 		          [&](const Void&) {
 			          if (logSystem->get()) {
-				          result =
-				              logSystem->get()->peekLogRouter(dbgid, beginVersion, routerTag, useSatellite, recoverAt);
+				          result = logSystem->get()->peekLogRouter(


Maybe LogRouter doesn't explicitly need to receive the list of participating log servers (receiving it as part of LogSystemConfig is probably good enough). I will need to look into that.

sbodagala · 2025-05-28T22:32:38Z

fdbserver/include/fdbserver/LogSystem.h

@@ -229,12 +229,15 @@ struct ILogSystem {
 		int fastReplies;
 		int unknownReplies;

+		bool returnEmptyIfStopped; // valid only when unicast is enabled


The source log server can return an empty version range only if this flag is set.

sbodagala · 2025-05-28T22:37:44Z

fdbserver/include/fdbserver/TagPartitionedLogSystem.actor.h

@@ -212,6 +213,13 @@ struct TagPartitionedLogSystem final : ILogSystem, ReferenceCounted<TagPartition
 	                     Optional<UID> debugID,
 	                     Optional<std::unordered_map<uint16_t, Version>> tpcvMap) final;

+	void maybeAdjustBestServer(int bestSet,


Please note that this API may need to be called in other peek calls too, thanks!

foundationdb-ci · 2025-05-28T22:54:50Z

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

Commit ID: 9a34167
Duration 0:37: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-05-28T23:16:43Z

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

Commit ID: 9a34167
Duration 0:59: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)

sbodagala added 7 commits March 12, 2025 19:42

If version vector is enabled then preserve all versions from "knownCo…

0ada5c9

…mmittedVersion" onwards in log server's disk queue.

- Address a review comment

4a348f4

- Make log servers preserve the disk queue positions of all versions

85c91b4

from known committed version onwards, when version vector is enabled.

sbodagala requested review from dlambrig and jzhou77 May 28, 2025 22:16

sbodagala marked this pull request as draft May 28, 2025 22:26

sbodagala commented May 28, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Make log servers return an empty version range only when it is correct to do so #12171

Make log servers return an empty version range only when it is correct to do so #12171

Uh oh!

sbodagala commented May 28, 2025

Uh oh!

sbodagala commented May 28, 2025

Uh oh!

foundationdb-ci commented May 28, 2025

Uh oh!

foundationdb-ci commented May 28, 2025

Uh oh!

foundationdb-ci commented May 28, 2025

Uh oh!

foundationdb-ci commented May 28, 2025

Uh oh!

foundationdb-ci commented May 28, 2025

Uh oh!

sbodagala May 28, 2025

Uh oh!

sbodagala May 28, 2025

Uh oh!

sbodagala May 28, 2025

Uh oh!

foundationdb-ci commented May 28, 2025

Uh oh!

foundationdb-ci commented May 28, 2025

Uh oh!

Uh oh!

Make log servers return an empty version range only when it is correct to do so #12171

Are you sure you want to change the base?

Make log servers return an empty version range only when it is correct to do so #12171

Uh oh!

Conversation

sbodagala commented May 28, 2025

Code-Reviewer Section

For Release-Branches

Uh oh!

sbodagala commented May 28, 2025

Uh oh!

foundationdb-ci commented May 28, 2025

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

Uh oh!

foundationdb-ci commented May 28, 2025

Result of foundationdb-pr on Linux RHEL 9

Uh oh!

foundationdb-ci commented May 28, 2025

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

Uh oh!

foundationdb-ci commented May 28, 2025

Result of foundationdb-pr-clang on Linux RHEL 9

Uh oh!

foundationdb-ci commented May 28, 2025

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

Uh oh!

sbodagala May 28, 2025

Choose a reason for hiding this comment

Uh oh!

sbodagala May 28, 2025

Choose a reason for hiding this comment

Uh oh!

sbodagala May 28, 2025

Choose a reason for hiding this comment

Uh oh!

foundationdb-ci commented May 28, 2025

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

Uh oh!

foundationdb-ci commented May 28, 2025

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

Uh oh!

Uh oh!