Skip to content

fix: correctly notify auto-cleaner when an item's TTL is shortened#206

Open
PiAreSquared wants to merge 1 commit into
jellydator:v3from
PiAreSquared:pi/fix-head-ttl-expiry
Open

fix: correctly notify auto-cleaner when an item's TTL is shortened#206
PiAreSquared wants to merge 1 commit into
jellydator:v3from
PiAreSquared:pi/fix-head-ttl-expiry

Conversation

@PiAreSquared
Copy link
Copy Markdown

Description

This PR fixes a bug where the background auto-cleaner fails to wake up early if an existing item's TTL is shortened, causing the item to live past its new expiration time. This seems to have been first reported in #204.

Root Cause

When Set() (or Get() with touch) is called on an existing item, the item's internal expiresAt is mutated first. Afterwards, updateExpirations() is called to reorder the heap and notify the auto-cleaner's timer channel.

Previously, updateExpirations attempted to snapshot the old closest expiration time after the item had already been mutated. If the updated item was already at the front of the expiration queue, oldExpiresAt perfectly matched the newly updated time. Because the method didn't detect a change in the root expiration time, it silently returned without notifying the timerCh, causing the auto-cleaner to oversleep.

Fix

  1. Modified the updateExpirations signature to take oldExpiresAt as a parameter
  2. Captured the top of the expQueue in set() and get() before mutating the item via item.touch() or item.update()
  3. Passed the strictly pre-mutated oldExpiresAt into updateExpirations

Testing

I updated the existing Test_Cache_updateExpirations to accommodate the signature change and added a new regression test. Test_Cache_ShortenedTTLNotifiesAutoCleane sets an item with a 1-hour TTL, immediately updates it to 10ms, and asserts that the auto-cleaner wakes up and evicts the item on time.

@davseby davseby assigned davseby and unassigned davseby Apr 26, 2026
@davseby davseby requested review from davseby and swithek April 26, 2026 16:40
@coveralls
Copy link
Copy Markdown

Coverage Report for CI Build 24898134592

Coverage increased (+0.003%) to 99.738%

Details

  • Coverage increased (+0.003%) from the base build.
  • Patch coverage: 16 of 16 lines across 1 file are fully covered (100%).
  • No coverage regressions found.

Uncovered Changes

No uncovered changes found.

Coverage Regressions

No coverage regressions found.


Coverage Stats

Coverage Status
Relevant Lines: 762
Covered Lines: 760
Line Coverage: 99.74%
Coverage Strength: 24.76 hits per line

💛 - Coveralls

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.

3 participants