Skip to content

fix: update entries already admitted to LRU2Cache - #16503

Open
zsewasdsdsd wants to merge 1 commit into
apache:3.3from
zsewasdsdsd:fix/lru2-promoted-put
Open

zsewasdsdsd wants to merge 1 commit into
apache:3.3from
zsewasdsdsd:fix/lru2-promoted-put

Conversation

@zsewasdsdsd

Copy link
Copy Markdown

Summary

Fixes #16500.

This PR fixes an issue in LRU2Cache.put() where updating a key that has already been promoted to the main cache does not replace its existing value.

Problem

LRU2Cache uses a two-write admission policy:

  1. The first put() records the key in the history cache (preCache).
  2. The second put() promotes the key into the main cache.
  3. Subsequent put() calls should update the existing cached value.

However, the current implementation only checks whether the key exists in preCache. Once a key has been promoted, it is removed from preCache, causing subsequent updates to be treated as new entries instead of updating the main cache.

As a result, callers may continue receiving stale cached values.

Changes

  • Check whether the key already exists in the main cache before applying the two-write admission policy.
  • Update existing entries while preserving the previous-value return behavior.
  • Preserve the existing locking mechanism and LRU eviction order.
  • Add regression tests covering promoted-entry updates, LRU ordering, and null-valued entries.

Verification

RED — Before the fix

  • Reproduced the issue against the original upstream Java implementation.
  • 3 expected regression failures; 1 existing-behavior check passed.

GREEN — After the fix

  • All 4 standalone regression checks passed.
  • Ran LRU2CacheTest using Maven on Windows with Java 21.
  • Tests run: 7
  • Failures: 0
  • Errors: 0
  • Skipped: 0
  • Maven result: BUILD SUCCESS

No full-repository test suite or distributed RPC integration tests were run.

Impact

This change ensures that already-admitted cache entries can be updated correctly without changing the admission policy for new keys or introducing new dependencies.

Preserve the two-write admission policy for new keys, but update
values already present in the main cache. Keep the existing lock
and access-ordered eviction behavior.

Add regressions for promoted-entry updates, previous values,
LRU eviction order, and null-valued entries.

Fixes apache#16500
@codecov-commenter

codecov-commenter commented Oct 13, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 58.96%. Comparing base (ae809de) to head (d419b72).

Additional details and impacted files
@@             Coverage Diff              @@
##                3.3   #16503      +/-   ##
============================================
- Coverage     60.92%   58.96%   -1.97%     
  Complexity       15       15              
============================================
  Files          1953     1953              
  Lines         89278    89280       +2     
  Branches      13476    13476              
============================================
- Hits          54390    52641    -1749     
- Misses        29313    30945    +1632     
- Partials       5575     5694     +119     
Flag Coverage Δ
integration-tests-java21 ?
integration-tests-java8 ?
samples-tests-java21 ?
samples-tests-java8 ?
unit-tests-java11 59.16% <100.00%> (-0.01%) ⬇️
unit-tests-java17 58.64% <100.00%> (-0.05%) ⬇️
unit-tests-java21 58.67% <100.00%> (+0.01%) ⬆️
unit-tests-java25 58.62% <100.00%> (+0.01%) ⬆️
unit-tests-java8 59.20% <100.00%> (+0.03%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

This branch has not been deployed

No deployments
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.

[Bug] LRU2Cache.put ignores updates for keys already promoted to the main cache

2 participants