Skip to content

[Tsavorite] Do not wait for a stopped size tracker in NeedToWaitForClose during AOF replay - #2175

Closed
Federico Laurianti (Laurianti) wants to merge 2 commits into
microsoft:mainfrom
Laurianti:fix-recovery-heap-overbudget-hang
Closed

Federico Laurianti (Laurianti) wants to merge 2 commits into
microsoft:mainfrom
Laurianti:fix-recovery-heap-overbudget-hang

Conversation

@Laurianti

@Laurianti Federico Laurianti (Laurianti) commented Sep 27, 2026 •

Copy link
Copy Markdown
Contributor

Root Cause

During AOF replay the LogSizeTracker has not been started yet. NeedToWaitForClose still asks it to evict: when IsBeyondSizeLimitAndCanEvict(addingPage: true) is true it calls Signal() and returns true, so TryAllocateRetryNow retries. If the log heap alone is over budget and the page count is below MaxAllocatedPageCount, IssueShiftAddress does not shift HeadAddress either, and the retry loop never ends. Recovery hangs before the server binds its port. It needs objects (heap) over budget, so string-only data is not affected.

#1932 added the !logSizeTracker.IsRunning guard to IssueShiftAddress for this reason; NeedToWaitForClose was left without it.

Description of Change

libs/storage/Tsavorite/cs/src/core/Allocator/AllocatorBase.cs: NeedToWaitForClose returns false when the size tracker is not running, as IssueShiftAddress already does. With a running tracker nothing changes.

Tests

  • RespAdminCommandsTests.SeAofRecoverObjectsOverHeapBudgetTest: 2,000 hashes of 2 KB on a lowMemory server with AOF, unclean dispose, then recover. Like SeSaveRecoverMultipleKeysTest, it calls server.Start() directly, so a regression shows up as a hang (without the change, --blame-hang-timeout 3m aborts it); with the change, all values are recovered.
  • SeAofRecover* and SeSaveRecover* (19 passed, 2 skipped), Tsavorite.test.recovery (246 passed), Tsavorite.test.hlog (596 passed), Garnet.test.extensions (536 passed), net10.0 Release on Windows, on current main.
  • The repro in AOF recovery hangs when objects put the log heap over the size tracker budget #2174 (30,000 HSET, docker kill, docker start) recovers in about 2 seconds with all keys.

Issues Fixed

Fixes #2174

Copilot AI balanced review requested due to automatic review settings September 27, 2026 13:50

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.

Copilot review overview

🟡 Changes recommended

The timeout leaves startup running while teardown concurrently disposes its allocator resources.

Review effort: Balanced
Findings: 1 Medium severity

Open (1)
What changed in this PR

Prevents AOF recovery from livelocking when object heap usage exceeds budget before the size tracker starts.

Changes:

  • Bypasses inactive size-tracker waits during page allocation.
  • Adds an object-heavy AOF recovery regression test.
File Description
AllocatorBase.cs Guards eviction signaling with IsRunning.
RespAdminCommandsTests.cs Tests recovery above the heap budget.

💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread test/standalone/Garnet.test/RespAdminCommandsTests.cs Outdated
@TedHartMS

Copy link
Copy Markdown
Contributor

Thanks for the contribution! This has been consolidated into PR #2181

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.

AOF recovery hangs when objects put the log heap over the size tracker budget

3 participants