Skip to content

Fix false cache hit accounting on initial MultiMapAsync execution - #2242

Open
Nacl-HCl wants to merge 1 commit into
DapperLib:mainfrom
Nacl-HCl:fix/multimap-async-cache-hit-2240
Open

Nacl-HCl wants to merge 1 commit into
DapperLib:mainfrom
Nacl-HCl:fix/multimap-async-cache-hit-2240

Conversation

@Nacl-HCl

Copy link
Copy Markdown

Description

Fixes #2240

When executing an asynchronous multi-map query (\MultiMapAsync), Dapper calls \GetCacheInfo(identity, param, command.AddToCache)\ to resolve the parameter reader and set up the command. It then delegated to \MultiMapImpl, which performed a second call to \GetCacheInfo\ for the same identity without passing the already-resolved \CacheInfo.

Because \GetCacheInfo\ invokes \RecordHit()\ upon finding an existing cache entry (\TryGetQueryCache), this second internal lookup caused the very first execution of an async multi-map query to record a hit count of 1 rather than 0.

As reported in #2240, because \CollectCacheGarbage\ collects entries with \hitCount <= 0, queries executed once via \MultiMapAsync\ survived cache garbage collection, which could accumulate one-off/interpolated multi-map queries over time.

Changes

  • Added optional \CacheInfo? cacheInfo = null\ parameter to both the generic and non-generic overloads of \MultiMapImpl.
  • In \MultiMapAsync, passed the resolved \info\ and \CommandDefinition.ForCallback(command.Parameters, command.Flags)\ into \MultiMapImpl.
  • In \MultiMapImpl, used \cacheInfo ?? GetCacheInfo(...).
  • Added unit tests in \SqliteTests.cs\ verifying that initial execution of both generic and types-based \MultiMapAsync\ queries yields a hit count of 0, and that subsequent executions correctly record reuse (hit count = 1).

Verification

  • Build completed with 0 errors across all target frameworks (
    et8.0,
    et10.0,
    etstandard2.0,
    et461, etc.).
  • Unit test \Issue2240_MultiMapAsync_CacheHitAccounting\ verified on both
    et8.0\ and
    et10.0.
  • All \SqliteTests\ passed with 0 failures.

MultiMapAsync calls GetCacheInfo to setup the command and parameter
reader. It then delegated to MultiMapImpl, which performed a redundant
GetCacheInfo call for the same identity. Because GetCacheInfo records a
cache hit on finding an existing entry, a single-execution async
multi-map query ended up with hitCount = 1 instead of 0, preventing
garbage collection by CollectCacheGarbage and causing one-off queries
to linger in the cache.

Pass the already resolved CacheInfo into MultiMapImpl so the first
execution retains hitCount = 0 and normal cache cleanup can collect
unreused queries.

Fixes DapperLib#2240

This branch has not been deployed

No deployments
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.

Async multi-map records cache reuse on first execution, preventing eviction of one-off SQL, occasionally leading to OOM

1 participant