Skip to content

Fix race in LFU maintenance for add-remove-add of the same key#533

Merged
bitfaster merged 12 commits into
mainfrom
users/alexpeck/updatestate
Dec 5, 2023
Merged

Fix race in LFU maintenance for add-remove-add of the same key#533
bitfaster merged 12 commits into
mainfrom
users/alexpeck/updatestate

Conversation

@bitfaster

@bitfaster bitfaster commented Dec 4, 2023

Copy link
Copy Markdown
Owner
  • Evict removes via KVP, instead of just key. This prevents incorrect removal of nodes added to the dictionary when there are two nodes in the write buffer/LRUs with the same key. For example:
    • Node 1 is added with key x
    • Node 1 is removed, now exists only in write buffer + LRU list
    • Node 2 is added with key x, and is also in the write buffer + LRU list
    • Maintenance previously found node 1, and deleted key x from the dictionary detaching node 2. Delete is now via KVP, so node 2 will not be deleted in this situation.
  • Check for removed nodes during EvictFromMain()
  • Evict() marks items as removed/deleted
  • Revise Trim logic to use LRU count instead of the dictionary. If TryRemove has been called, the dictionary can contain fewer items than the LRU lists. First call Maintenance() to flush buffers and mitigate this.
  • Enable node detachment integrity check as part of soak tests

Comment thread BitFaster.Caching/Lfu/ConcurrentLfuCore.cs
@coveralls

coveralls commented Dec 5, 2023

Copy link
Copy Markdown

Coverage Status

coverage: 99.181% (-0.06%) from 99.237%
when pulling 57d8d97 on users/alexpeck/updatestate
into 9441fa9 on main.

@bitfaster bitfaster changed the title Handle removed nodes as part of eviction Fix race in LFU maintenance for add-remove-add of the same key Dec 5, 2023
@bitfaster bitfaster marked this pull request as ready for review December 5, 2023 02:47
@bitfaster bitfaster merged commit eab6108 into main Dec 5, 2023
@bitfaster bitfaster deleted the users/alexpeck/updatestate branch December 5, 2023 03:19
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.

2 participants