Skip to content

[ISSUE #10572] Add dual TTL policy for tiered storage read-ahead cache#10573

Open
Houlong66 wants to merge 1 commit into
apache:developfrom
Houlong66:local/dual-ttl-fetcher
Open

[ISSUE #10572] Add dual TTL policy for tiered storage read-ahead cache#10573
Houlong66 wants to merge 1 commit into
apache:developfrom
Houlong66:local/dual-ttl-fetcher

Conversation

@Houlong66

Copy link
Copy Markdown
Contributor

Which Issue(s) This PR Fixes

Brief Description

This PR adds a native dual-TTL policy for the tiered storage read-ahead cache.

The new default behavior keeps unread prefetched entries for 180000ms after creation, then switches entries to a shorter 10000ms TTL after they are read. This gives consumers more time to consume read-ahead batches while allowing already-read entries to be released sooner.

Main changes:

  • Add readAheadCacheCreateExpireDuration and readAheadCacheAfterReadExpireDuration to MessageStoreConfig.
  • Keep readAheadCacheExpireDuration as the legacy single-TTL fallback.
  • Build the read-ahead cache with Caffeine Expiry for create/read/update-specific expiration.
  • Add focused tests for default resolution, fallback behavior, create/read/update expiration semantics, and inverted TTL configuration.
  • Update the tieredstore README configuration table.

How Did You Test This Change?

export JAVA_HOME=/Library/Java/JavaVirtualMachines/openjdk-11.jdk/Contents/Home
mvn -pl tieredstore test -Dtest=MessageStoreFetcherImplCacheTest
mvn -pl tieredstore test -DfailIfNoTests=false

@Houlong66 Houlong66 marked this pull request as ready for review July 2, 2026 06:30
@codecov-commenter

codecov-commenter commented Jul 2, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 83.67347% with 8 lines in your changes missing coverage. Please review.
✅ Project coverage is 48.16%. Comparing base (41a3a35) to head (2eaa01e).
⚠️ Report is 60 commits behind head on develop.

Files with missing lines Patch % Lines
...etmq/tieredstore/core/MessageStoreFetcherImpl.java 79.48% 5 Missing and 3 partials ⚠️
Additional details and impacted files
@@              Coverage Diff              @@
##             develop   #10573      +/-   ##
=============================================
- Coverage      49.01%   48.16%   -0.85%     
+ Complexity     13485    13418      -67     
=============================================
  Files           1376     1378       +2     
  Lines         100527   100864     +337     
  Branches       12983    13047      +64     
=============================================
- Hits           49274    48584     -690     
- Misses         45251    46317    +1066     
+ Partials        6002     5963      -39     

☔ 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.

@RockteMQ-AI RockteMQ-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.

Review by github-manager-bot

Summary

Introduces a dual-TTL policy for the tiered storage read-ahead cache: unread entries use a longer createTtl (default 180s), then after first read switch to a shorter afterReadTtl (default 10s). Falls back to legacy single-TTL expireAfterAccess when both dual TTLs are disabled.

Findings

  • [Info] MessageStoreFetcherImpl.java — The DualTtlExpiry implementing Caffeine's Expiry interface is clean and correct. expireAfterCreate/expireAfterUpdate return createNanos, expireAfterRead returns afterReadNanos — this gives the intended "long before read, short after read" semantics.
  • [Info] resolveReadAheadCacheTtl() has solid fallback logic: both disabled → legacy; one disabled → use readAheadCacheExpireDuration as fallback for the missing one; both partially invalid → legacy. Well-covered by tests.
  • [Info] TieredMessageStore.createFetcher() is extracted as a protected method to allow subclass override. The Javadoc correctly warns about virtual dispatch from constructor — good defensive documentation.
  • [Info] memoryMaxSize changed from private to protected — necessary for the subclass override pattern. Acceptable.
  • [Warning] DualTtlExpiry.expireAfterUpdate resets to createNanos on update. This means if a cache entry is re-put (same key), it gets the full create TTL again. The test dualTtlCache_readThenRePutResetsToCreateTtl confirms this is intentional. Just flagging this as a behavioral note: repeated writes to the same offset will keep entries alive at the longer TTL.
  • [Info] The warning log when afterReadTtl >= createTtl is a nice touch — helps operators catch misconfiguration.
  • [Info] Test coverage is comprehensive: default config, disabled dual TTL, partial fallback, invalid partial fallback, create-TTL expiry, after-read-TTL expiry, re-put reset, and the isAfterReadNotShorter edge case. The FakeTicker approach is clean.

Suggestions

  • The README table still shows the old readAheadCacheExpireDuration description as "Read-ahead cache expiration time". Consider updating it to clarify it's now the legacy fallback, alongside the two new columns.

Automated review by github-manager-bot

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.

[Enhancement] Add dual TTL policy for tiered storage read-ahead cache

3 participants