Skip to content

Tighten up Recovery/replay/eviction interaction with sizeTracker, including at shutdown, to fix intermittent Garnet test hangs#1932

Open
TedHartMS wants to merge 8 commits into
mainfrom
tedhar/sesr-mkt
Open

Tighten up Recovery/replay/eviction interaction with sizeTracker, including at shutdown, to fix intermittent Garnet test hangs#1932
TedHartMS wants to merge 8 commits into
mainfrom
tedhar/sesr-mkt

Conversation

@TedHartMS

Copy link
Copy Markdown
Contributor

This pull request introduces important changes to the log size tracking and eviction mechanisms in the Tsavorite storage engine, focusing on improving concurrency safety and preventing hangs during recovery and disposal. The main updates ensure that the background resizer does not interfere with recovery, that resource cleanup is orderly, and that eviction is handled correctly depending on whether the resizer is running.

Concurrency and Recovery Safety:

  • The background size-tracker resizer is no longer started before recovery; instead, it is started after recovery completes to prevent concurrency issues that could lead to hangs during log recovery. The resizer is only responsible for eviction when running; otherwise, eviction is handled synchronously during recovery. (StoreWrapper.cs, AllocatorBase.cs) [1] [2]
  • The disposed field in AllocatorBase is now volatile, and eviction/flush wait loops check disposed to avoid spinning indefinitely during shutdown or disposal. (AllocatorBase.cs) [1] [2] [3] [4]

Orderly Shutdown and Resource Management:

  • The Dispose method in AllocatorBase now stops the size-tracker resizer and waits for it to exit before releasing resources, ensuring the resizer does not access freed resources and preventing potential race conditions. (AllocatorBase.cs) [1] [2]
  • LogSizeTracker now tracks the resizer task and provides a robust Stop(wait: true) mechanism that waits for the task to complete, even if the task was never started due to a pre-canceled token. (LogSizeTracker.cs) [1] [2] [3]

Eviction Logic Improvements:

  • The eviction logic in IssueShiftAddress now checks if the resizer is running (IsRunning) before deciding whether to evict synchronously, ensuring correct eviction behavior during recovery and normal operation. (AllocatorBase.cs, LogSizeTracker.cs) [1] [2]

These changes collectively improve the reliability and correctness of log size management, especially during recovery and shutdown scenarios.

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 aims to eliminate intermittent hangs in Garnet/Tsavorite by tightening the interaction between recovery/AOF replay, eviction, and the log size-tracker resizer—especially during shutdown—so background resizing cannot race recovery or touch torn-down resources.

Changes:

  • Deferred starting the background size-tracker resizer until after checkpoint recovery completes, to avoid recovery/resizer races (StoreWrapper.cs).
  • Hardened allocator shutdown and wait loops by stopping the resizer earlier during AllocatorBase.Dispose() and bailing out of flush/eviction waits once disposal begins (AllocatorBase.cs).
  • Extended LogSizeTracker with IsRunning and a retained resizerTask to improve eviction decisions when the resizer is not active (LogSizeTracker.cs).

PR metadata (title/description) alignment:

  • The description matches the intent and the code changes at a high level, but the current LogSizeTracker.Stop(wait: true) implementation still has a race/semantic gap that can prevent the intended “wait for resizer exit” guarantee (see stored review comment).

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File Description
libs/storage/Tsavorite/cs/src/core/Index/Common/LogSizeTracker.cs Track resizer task state (IsRunning, resizerTask) and adjust start/stop behavior to avoid shutdown hangs.
libs/storage/Tsavorite/cs/src/core/Allocator/AllocatorBase.cs Stop/wait for resizer earlier in dispose; add disposed checks to avoid indefinite spin-waits; sync-evict when resizer not running.
libs/server/StoreWrapper.cs Avoid starting the resizer during checkpoint recovery; rely on synchronous eviction pre-resizer-start.

Comment thread libs/storage/Tsavorite/cs/src/core/Index/Common/LogSizeTracker.cs
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