Skip to content

Fix peekLogRouter() born-exhausted cursor when begin == oldEnd#13238

Merged
saintstack merged 1 commit into
apple:mainfrom
saintstack:oneliner
May 21, 2026
Merged

Fix peekLogRouter() born-exhausted cursor when begin == oldEnd#13238
saintstack merged 1 commit into
apple:mainfrom
saintstack:oneliner

Conversation

@saintstack

@saintstack saintstack commented May 19, 2026

Copy link
Copy Markdown
Contributor

When begin == oldEnd, the code falls through and creates a SetPeekCursor with begin == end. This cursor is born exhausted -- getMore() returns immediately with no data and no advancement, which can cause LogRouters to hang.

The >= check catches this case and skips the old epoch (same path as begin > oldEnd), preventing the born-exhausted cursor from being created.

This is a defensive correctness fix -- a cursor with begin == end is always wrong. The case is rare and was not directly observed in traced failures, but closing this edge case is warranted.

Here is a 100k run 20260518-223352-stack_one_liner-58efc5e594c8620e compressed=True data_size=37007600 duration=3861738 ended=100000 fail=4 fail_fast=10 max_runs=100000 pass=99996 priority=100 remaining=0 runtime=0:52:28 sanity=False started=100000 stopped=20260518-232620 submitted=20260518-223352 timeout=5400 username=stack_one_liner

Two failures are:

RandomSeed="1027964057" SourceVersion="df7ce9300dd36343d59a4e66c6a1f9622cffdddb" Time="1779145186" BuggifyEnabled="0" DeterminismCheck="0" FaultInjectionEnabled="1" TestFile="tests/fast/Sideband.toml"

and

RandomSeed="762784548" SourceVersion="df7ce9300dd36343d59a4e66c6a1f9622cffdddb" Time="1779145515" BuggifyEnabled="0" DeterminismCheck="0" FaultInjectionEnabled="1" TestFile="tests/fast/Sideband.toml"

These are from an existing 'live-TLog' hang I've been following (multi-region, timed out at 5401s, 0 TracedTooManyLines). I just ran 500k against main and 4 of these Sideband failures showed up there.

This issue comes of the work over in #13228 where I had trouble proving an improvement. This one-liner extracts from that work an obvious (to-me) improvement.

Copilot AI reviewed May 19, 2026
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR defensively fixes an edge case in LogSystem::peekLogRouter() where begin == oldEnd could previously fall through and construct a SetPeekCursor over an empty range (begin == end), producing a cursor that is immediately exhausted and can cause LogRouter replay to stall/hang.

Changes:

  • Treat begin == oldEnd the same as begin > oldEnd when iterating old epochs, skipping that old epoch instead of constructing a SetPeekCursor with an empty range.
  • Add an explanatory comment and update the associated trace event emitted when skipping an old epoch whose range is already fully behind begin.

@foundationdb-ci

foundationdb-ci commented May 19, 2026

Copy link
Copy Markdown
Contributor

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

  • Commit ID: df7ce93
  • Duration 0:23:51
  • 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

foundationdb-ci commented May 19, 2026

Copy link
Copy Markdown
Contributor

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

  • Commit ID: df7ce93
  • Duration 0:34: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

foundationdb-ci commented May 19, 2026

Copy link
Copy Markdown
Contributor

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

  • Commit ID: df7ce93
  • Duration 0:44:01
  • 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

foundationdb-ci commented May 19, 2026

Copy link
Copy Markdown
Contributor

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

  • Commit ID: df7ce93
  • Duration 0:45:22
  • 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

foundationdb-ci commented May 19, 2026

Copy link
Copy Markdown
Contributor

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

  • Commit ID: df7ce93
  • Duration 1:06:04
  • 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

foundationdb-ci commented May 19, 2026

Copy link
Copy Markdown
Contributor

Result of foundationdb-pr-clang on Linux RHEL 9

  • Commit ID: df7ce93
  • Duration 0:53:47
  • 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

foundationdb-ci commented May 19, 2026

Copy link
Copy Markdown
Contributor

Result of foundationdb-pr on Linux RHEL 9

  • Commit ID: df7ce93
  • Duration 1:02: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)

@spraza

spraza commented May 19, 2026

Copy link
Copy Markdown
Collaborator

@tclinkenbeard-oai can you also comment on this? I saw that you explicitly changed this the other way in your PR commit: 5a63d11. So curious what issue you saw there.

@saintstack

saintstack commented May 19, 2026

Copy link
Copy Markdown
Contributor Author

Thank you for the review @sbodagala . Yeah, @tclinkenbeard-oai , interested in your opinion on this one. Thank you.

@spraza spraza mentioned this pull request May 19, 2026
Merged
5 tasks
@saintstack

saintstack commented May 19, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

RandomSeed="1663748750" SourceVersion="df7ce9300dd36343d59a4e66c6a1f9622cffdddb" Time="1779156188" BuggifyEnabled="0" DeterminismCheck="0" FaultInjectionEnabled="1" TestFile="tests/slow/CommitBug.toml"

Test times out. Test doesn't get to start. Not seemingly related to TLog and peeking...

@saintstack saintstack closed this May 19, 2026
@saintstack saintstack reopened this May 19, 2026
@foundationdb-ci

foundationdb-ci commented May 19, 2026

Copy link
Copy Markdown
Contributor

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

  • Commit ID: df7ce93
  • Duration 0:23:34
  • 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

foundationdb-ci commented May 19, 2026

Copy link
Copy Markdown
Contributor

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

  • Commit ID: df7ce93
  • Duration 0:33:40
  • 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

foundationdb-ci commented May 19, 2026

Copy link
Copy Markdown
Contributor

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

  • Commit ID: df7ce93
  • Duration 0:44:38
  • 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

foundationdb-ci commented May 20, 2026

Copy link
Copy Markdown
Contributor

Result of foundationdb-pr on Linux RHEL 9

  • Commit ID: df7ce93
  • Duration 0:46:35
  • 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

foundationdb-ci commented May 20, 2026

Copy link
Copy Markdown
Contributor

Result of foundationdb-pr-clang on Linux RHEL 9

  • Commit ID: df7ce93
  • Duration 0:50:30
  • 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

foundationdb-ci commented May 20, 2026

Copy link
Copy Markdown
Contributor

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

  • Commit ID: df7ce93
  • Duration 1:09: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)
  • Cluster Test Logs zip file of the test logs (available for 30 days)

@foundationdb-ci

foundationdb-ci commented May 20, 2026

Copy link
Copy Markdown
Contributor

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

  • Commit ID: df7ce93
  • Duration 2:07: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)

Fix peekLogRouter() born-exhausted cursor when begin == oldEnd
5214284 
When begin == oldEnd, the code falls through and creates a SetPeekCursor
with begin == end. This cursor is born exhausted -- getMore() returns
immediately with no data and no advancement, which can cause LogRouters
to hang.

The >= check catches this case and skips the old epoch (same path as
begin > oldEnd), preventing the born-exhausted cursor from being created.

This is a defensive correctness fix -- a cursor with begin == end is
always wrong. The case is rare and was not directly observed in traced
failures, but closing this edge case is warranted.

Validated with 100k simulation runs across AutomaticIdempotency,
InventoryTestSomeWrites, and MaxGrvQueueDelay with no obvious
regressions.
@saintstack

saintstack commented May 21, 2026

Copy link
Copy Markdown
Contributor Author

Rebased.

@saintstack

saintstack commented May 21, 2026

Copy link
Copy Markdown
Contributor Author

This good by you @tclinkenbeard-oai ? Thanks.

@tclinkenbeard-oai

tclinkenbeard-oai commented May 21, 2026

Copy link
Copy Markdown
Collaborator

Yeah, this change LGTM, thank you

@foundationdb-ci

foundationdb-ci commented May 21, 2026

Copy link
Copy Markdown
Contributor

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

  • Commit ID: 5214284
  • Duration 0:24:24
  • 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

foundationdb-ci commented May 21, 2026

Copy link
Copy Markdown
Contributor

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

  • Commit ID: 5214284
  • Duration 0:45: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

foundationdb-ci commented May 21, 2026

Copy link
Copy Markdown
Contributor

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

  • Commit ID: 5214284
  • Duration 0:46: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

foundationdb-ci commented May 21, 2026

Copy link
Copy Markdown
Contributor

Result of foundationdb-pr-clang on Linux RHEL 9

  • Commit ID: 5214284
  • Duration 0:51:37
  • 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

foundationdb-ci commented May 21, 2026

Copy link
Copy Markdown
Contributor

Result of foundationdb-pr on Linux RHEL 9

  • Commit ID: 5214284
  • Duration 0:53: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

foundationdb-ci commented May 21, 2026

Copy link
Copy Markdown
Contributor

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

  • Commit ID: 5214284
  • Duration 1:02:39
  • 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

foundationdb-ci commented May 21, 2026

Copy link
Copy Markdown
Contributor

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

  • Commit ID: 5214284
  • Duration 1:07:38
  • 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)

@saintstack saintstack merged commit 4cc183b into apple:main May 21, 2026
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot Copilot left review comments
@sbodagala sbodagala sbodagala approved these changes
@tclinkenbeard-oai tclinkenbeard-oai tclinkenbeard-oai approved these changes
@spraza spraza Awaiting requested review from spraza
+1 more reviewer
@Ronitsabhaya75 Ronitsabhaya75 Ronitsabhaya75 approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@saintstack @foundationdb-ci @spraza @tclinkenbeard-oai @sbodagala @Ronitsabhaya75