Skip to content

Fix OnMiss not firing in all cases in Distributed Cache - #211

Merged
jodydonetti merged 2 commits into
ZiggyCreatures:mainfrom
ConMur:FixDistributedCacheOnMiss
Mar 30, 2024
Merged

jodydonetti merged 2 commits into
ZiggyCreatures:mainfrom
ConMur:FixDistributedCacheOnMiss

Conversation

@ConMur

@ConMur ConMur commented Mar 11, 2024

Copy link
Copy Markdown
Contributor

In the current implementation of DistributedCacheAccessor (both Sync and Async) the only time _events.OnMiss fires is when

  1. The de-serialized entry is null
  2. The de-serialization fails

The bug is in case 1. The code currently checks that the value returned from the IDistributedCache is non null and returns early if it is null. This means we never start de-serialization and therefore never call _events.OnMiss. This leads to inaccurate distributed cache miss counters.

To fix this I added _events.OnMiss to the if block that checks the result from the IDistributedCache. This will cause the OnMiss event to also be triggered in the case of an exception from the IDistributedCache. If this behavior is not desired I can update the code to only fire the event when the result from the IDistributedCache is null.

@ConMur
ConMur marked this pull request as ready for review March 11, 2024 18:29
@jodydonetti

jodydonetti commented Mar 19, 2024 •

Copy link
Copy Markdown
Collaborator

Hi @ConMur and thanks for using FusionCache!

Sorry for the delay but I was at the MVP Global Summit and was not able to look into this.
You seem to have a point: I'll look into it asap and will let you know.

Thanks!

@jodydonetti

Copy link
Copy Markdown
Collaborator

I'm looking into this, will update soon.

@jodydonetti jodydonetti self-assigned this Mar 30, 2024
@jodydonetti jodydonetti added the bug Something isn't working label Mar 30, 2024
@jodydonetti jodydonetti added this to the v1.0.1 milestone Mar 30, 2024

@jodydonetti jodydonetti left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM

@jodydonetti
jodydonetti merged commit def5241 into ZiggyCreatures:main Mar 30, 2024
@ConMur
ConMur deleted the FixDistributedCacheOnMiss branch April 1, 2024 21:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants