Skip to content

Log exceptions escaping OnPagesClosedWorker - #2025

Merged
Tiago Nápoli (tiagonapoli) merged 2 commits into
microsoft:mainfrom
tiagonapoli:tiagonapoli/log-close-pipeline-exceptions
Aug 5, 2026
Merged

Log exceptions escaping OnPagesClosedWorker#2025
Tiago Nápoli (tiagonapoli) merged 2 commits into
microsoft:mainfrom
tiagonapoli:tiagonapoli/log-close-pipeline-exceptions

Conversation

@tiagonapoli

@tiagonapoli Tiago Nápoli (tiagonapoli) commented Aug 5, 2026

Copy link
Copy Markdown
Collaborator

Summary

Adds logging for exceptions escaping OnPagesClosedWorker. Observability only — no behavior change. Catch, log via the allocator's existing logger, rethrow.

Why

OngoingCloseUntilAddress is a work-ownership token. From OnPagesClosed:

if (_ongoingCloseUntilAddress >= newSafeHeadAddress) break;   // "someone owns my range"
if (CAS(ref OngoingCloseUntilAddress, newSafeHeadAddress, _ongoing) == _ongoing) {
    if (_ongoingCloseUntilAddress == 0)   // worker spawned ONLY on the 0 -> non-zero transition
        OnPagesClosedWorker();
    return;
}

A worker is spawned only on the 0 → non-zero transition, and the only normal exit from the worker loop is CAS(ref OngoingCloseUntilAddress, 0, closeEndAddress). So an exception unwinding the worker leaves the token non-zero with no live worker — a state the pipeline cannot leave. Every later OnPagesClosed sees a non-zero token, assumes an owner is running, extends the token and starts nothing. ClosedUntilAddress never advances again, and the unbounded while (...) Thread.Yield() waits depending on it spin at 100% of a core indefinitely.

Evidence

A node had a thread pegged inside ResetCore's wait for ClosedUntilAddress during a replica attach:

OS Thread Id: 0x1a1cd

System.Threading.Thread.YieldInternal()
AllocatorBase`2+<>c__DisplayClass54_2[...].<ResetCore>b__1()   Allocator/AllocatorBase.cs @ 407
LightEpoch.Drain(Int64)                                        Epochs/LightEpoch.cs @ 497
LightEpoch.BumpCurrentEpoch(System.Action)                     Epochs/LightEpoch.cs @ 430
AllocatorBase`2[...].ResetCore()                               Allocator/AllocatorBase.cs @ 389
AllocatorBase`2[...].Reset()                                   Allocator/AllocatorBase.cs @ 331
TsavoriteKV`2[...].Reset()                                     Index/Recovery/Recovery.cs @ 419
DatabaseManagerBase.ResetDatabase(GarnetDatabase)              libs/server/Databases/DatabaseManagerBase.cs @ 270
StoreWrapper.Reset(Int32)                                      libs/server/StoreWrapper.cs @ 537
ReplicationManager...g__ReplicaSyncAttachTaskAsync|0>d.MoveNext()   libs/cluster/.../ReplicaDiskbasedSync.cs @ 146

Allocator state from the dump: ClosedUntilAddress = 16,777,216 (frozen), OngoingCloseUntilAddress = 2,031,377,992, SafeHeadAddress = 2,031,377,992 — 1.88 GiB of log that could never be closed.

The logs from that node contain no Tsavorite entries at all, so which exception killed the worker is unknowable after the fact. That is the gap this PR closes.

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

Adds allocator-level logging for unhandled exceptions escaping the OnPagesClosedWorker background pipeline, improving Tsavorite observability of unrecoverable “page closing stalled” failures without altering control flow (exceptions are still rethrown).

Changes:

  • Wrap OnPagesClosedWorker in a try/catch that logs a critical message with key allocator state and rethrows.
  • Extract the existing worker loop into OnPagesClosedWorkerCore() to keep the wrapper minimal.

Comment thread libs/storage/Tsavorite/cs/src/core/Allocator/AllocatorBase.cs Outdated
Tiago Martins Napoli and others added 2 commits August 5, 2026 13:10
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: c743170a-5362-4f06-b0a3-3f1e3c40017a
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
@tiagonapoli
Tiago Nápoli (tiagonapoli) force-pushed the tiagonapoli/log-close-pipeline-exceptions branch from 2e40d2f to 71905f5 Compare August 5, 2026 20:10
@tiagonapoli
Tiago Nápoli (tiagonapoli) merged commit 9f2effa into microsoft:main Aug 5, 2026
416 of 417 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants