Skip to content

fix: preserve LRU overwrite recency across delayed reads - #283

Open
perfloop[bot] wants to merge 2 commits into
mainfrom
perfloop-pr-open-z2sgvjg2zj
Open

perfloop[bot] wants to merge 2 commits into
mainfrom
perfloop-pr-open-z2sgvjg2zj

Conversation

@perfloop

@perfloop perfloop Bot commented Oct 8, 2026 •

Copy link
Copy Markdown
Contributor

The access updater no longer lets a delayed automatic read replace a later LRU timestamp published by an overwrite. This prevents the written key from moving ahead of older eviction candidates.

The guard applies only to admitted automatic updates in capped LRU. Storage.Get and Storage.GetWithVersion share the updater; FIFO and uncapped storage do not use it. The guard skips only when the current back-reference names the same key and its timestamp-derived bucket, signed nanosecond field, separators, and key are valid. Missing or malformed entries still follow the existing refresh path. Buffer-full drops, delay-gated reads, and same-key mutations between the flush lookup and commit remain outside the guarantee. Explicit SetAccessTime can still set an older timestamp. No public API, configuration, or index format changes.

TestAccessUpdater_DelayedReadDoesNotUndoNewerOverwrite holds a successful Get update until a successful Put commits in a later second, with an equal-sized key between the read and write times. It verifies both the t3 back-reference and the ordered row it names before ordinary LRU eviction. A same-batch later-index update confirms the access-update write committed. The test also checks that newer automatic reads advance recency and explicit older SetAccessTime values remain possible.

TestAccessUpdater_RewritesMalformedCurrentIndex verifies the old refresh behavior for malformed prefixes, buckets—including a valid bucket hour that disagrees with the timestamp—and separators. The frequency of this schedule in deployments remains unmeasured.

make all, make test, make test-integration-cleaner, make test-integration TEST=TestIntegration_Objects/ObjectsSuite/Test_Objects_LRUEviction, make test-e2e-eviction, make lint, make license-check, storage go vet ./..., and the targeted storage race test passed. The full integration and E2E suites were not run. make test-integration-short reached its configured 30-second package timeout on both revisions. The accepted-base run was in tests/integration: TestIntegration_Objects/ObjectsSuite/Test_Objects_MixedSizes; the final run was in tests/integration: TestIntegration_Objects/ObjectsSuite/Test_Objects_MixedTTL, during its two-second pre-read wait. These are suite-timeout locations, not test assertion failures. The Objects LRU eviction subtest had already run; its scoped regular integration check passed. The comparison runner is retained proof support and is not part of this pull request; the in-package regression and preservation tests ship.

The test-only comparison at 5f4ed5ac3aed0c61d5f669cf1e3b70d57cfaea13 reports violated for the asserted behavior.

The final source reports satisfied for the same assertion.


Generated by Perfloop.


Note

Medium Risk
Changes LRU index flush semantics for capped storage; incorrect logic could skew eviction, but the guard is limited to automatic updates with validated index entries.

Overview
Fixes a race where a delayed automatic read (from Get) could flush an older access timestamp and move a key ahead of a newer overwrite in the LRU index, causing wrong eviction under disk caps.

Access updates from UpdateNow are now tagged automatic. On flush, if the persisted bucketed access entry is well-formed and newer than the queued read time, the updater skips rewriting that key so overwrite recency is preserved. Explicit Update / SetAccessTime paths are unchanged. parseWellFormedBucketedAccessKey gates that comparison so malformed index rows still go through the normal refresh/delete path.

Adds regression tests for read-after-overwrite ordering, eviction behavior, newer reads still advancing recency, and malformed index repair.

Reviewed by Cursor Bugbot for commit 8b49514. Bugbot is set up for automated code reviews on this repo. Configure here.

Signed-off-by: Perfloop Agent <agent@perfloop.ai>
Signed-off-by: Perfloop Agent <agent@perfloop.ai>
@greptile-apps

greptile-apps Bot commented Oct 8, 2026 •

Copy link
Copy Markdown

RetriggerConfidence Score: 5/5

[Medium risk] Changes how the LRU cache tracks access times for eviction.

The PR appears safe to merge within its stated guarantee.

What we checked:

  • Explicit older times still work: Update leaves automatic false. Only automatic updates enter the new guard.
  • Malformed entries still refresh: parseWellFormedBucketedAccessKey rejects bad formats and bucket mismatches. The skip requires a valid entry naming the same key.

Summary

This PR prevents delayed automatic reads from replacing a newer stored LRU timestamp.

  • Explicit SetAccessTime updates still bypass the new guard.
  • Malformed stored entries still follow the existing refresh path.
  • Added tests check overwrite recency, eviction order, newer reads, and malformed entries.

perfloop[bot] explicitly limits the guarantee to admitted updates and writes completed before the flush lookup. Buffer drops, delay-gated reads, and same-key changes between lookup and commit remain outside it.

Review checks were read-only; no tests were run.

Reviews (1) · Last reviewed commit: "fix: keep automatic LRU reads from undoi..." · Reviewed by Greptile

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.

1 participant