fw: drop evicted entries from CsLRU location index - #213
Open
Harsh23Kashyap wants to merge 1 commit into
Open
Harsh23Kashyap wants to merge 1 commit into
Harsh23Kashyap wants to merge 1 commit into
Conversation
CsLRU.EvictEntries removed the evicted entry from its queue but left the entry in the locations map, because the CS-side erase callback bypasses BeforeErase. Every eviction leaked one map entry and one list element, so a forwarder running at CS capacity grew the index without bound. Delete the evicted index from locations on eviction. Add a regression test that inserts 100 entries at capacity 5 and checks the index shrinks with the queue.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #210.
Summary
CsLRU.EvictEntries removed the evicted entry from the LRU queue but never deleted its index from the locations map. The erase goes through eraseCsDataFromReplacementStrategy, which does not call BeforeErase, so no other path cleans it up. A forwarder running at content-store capacity leaks one map entry and one orphaned list element per eviction, growing the index without bound for the life of the process.
The fix deletes the evicted index from locations in EvictEntries. This is separate from #154, which tracks buffer allocations in the face link service.
Repro evidence
Go 1.26.6, linux/amd64, ndnd main @ d51682b. CS capacity 5, 100 distinct Data packets inserted.
Regression test against unpatched main:
Same test against this branch:
Testing