Skip to content

OAK-12214 : segment cache notify L2 on L1 hits to keep frequency/recency accurate#2903

Merged
rishabhdaim merged 7 commits into
trunkfrom
OAK-12214
May 19, 2026
Merged

OAK-12214 : segment cache notify L2 on L1 hits to keep frequency/recency accurate#2903
rishabhdaim merged 7 commits into
trunkfrom
OAK-12214

Conversation

@rishabhdaim
Copy link
Copy Markdown
Contributor

No description provided.

@rishabhdaim
Copy link
Copy Markdown
Contributor Author

rishabhdaim commented May 18, 2026

Build is failing due to resource leakage in DocumentNodeStoreIT test. To be fixed by : #2905

* L1-to-L2 propagation; downstream code that constructed ids with a {@code Runnable} must be updated.
*/
public SegmentId(@NotNull SegmentStore store, long msb, long lsb, @NotNull Runnable onAccess) {
public SegmentId(@NotNull SegmentStore store, long msb, long lsb, @NotNull Consumer<SegmentId> onAccess) {
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.

Not backwards compatible public function.

}

@Override
public void putSegment(@NotNull Segment segment) {
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.

Also flagged, but not related to this PR: calling putSegment twice for the same id might over-counts stats.currentWeight.

Copy link
Copy Markdown
Contributor

@jsedding jsedding left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me.

If the fix proves effective (which I have no doubt), I think we should remove the memoization (aka L1 cache) entirely.

registerCloseable(store);

// OAK-12214: bug-fix toggle (default on) so L2 eviction policy sees L1 memoised hits
registerCloseable(cfg.getWhiteboard().register(FeatureToggle.class,
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.

Maybe better would be to use utility org.apache.jackrabbit.oak.spi.toggle.Feature#newFeature

public abstract void recordHit(@NotNull SegmentId id);

/**
* Feature toggle name to enable embedded verification for full GC mode for Mongo Document Store
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.

????

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this file is outdated. latest one doesn't have this wrong comment.

* @param lsb least significant bits of this id
* @param onAccess callback invoked whenever the locally memoised segment is accessed
* ({@link #getSegment()}); receives {@code this} (e.g. to notify {@link SegmentCache}).
* <p><strong>API note (Oak 2.1, OAK-12214):</strong> this parameter type changed from
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.

Oak 2.1? Should be 2.2.0, right?

rishabhdaim and others added 6 commits May 19, 2026 09:10
…ency accurate

L1 memoization in SegmentId serves most segment reads without touching L2.
This means W-TinyLFU admission sketches and LRU recency lists never see hot
segments, and the eviction policy treats them as cold — potentially evicting
them under pressure.

SegmentCache.recordHit(SegmentId) now calls cache.getIfPresent(id) to
register the access with the L2 backing store. The behaviour is controlled
by feature toggle FT_NOTIFY_L2_OAK-12214 (enabled by default). Also adds
cleanUp() to flush pending Caffeine maintenance before reading eviction stats.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…s internal frequency updated such that we don't evict hot segments
@sonarqubecloud
Copy link
Copy Markdown

@rishabhdaim rishabhdaim merged commit d6d5c6f into trunk May 19, 2026
4 checks passed
@rishabhdaim rishabhdaim deleted the OAK-12214 branch May 19, 2026 07:56
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.

5 participants