Skip to content

fix: treat Redis deserialize failures as cache misses and enable the MethodRouting cache#2859

Merged
simonredfern merged 3 commits into
OpenBankProject:developfrom
hongwei1:fix/stability-batch3
Jul 13, 2026
Merged

fix: treat Redis deserialize failures as cache misses and enable the MethodRouting cache#2859
simonredfern merged 3 commits into
OpenBankProject:developfrom
hongwei1:fix/stability-batch3

Conversation

@hongwei1

Copy link
Copy Markdown
Contributor

Summary

Two commits, ordered on purpose:

1. fix(cache) — Redis deserialization failure poisoned the cache

Redis.deserialize returned the sentinel "NONE".asInstanceOf[T] when Kryo failed to decode cached bytes (corrupt entry, class-shape change across a redeploy, Kryo registration drift). scalacache treated that sentinel as a valid HIT, so every caller expecting the real type got a ClassCastException for the whole TTL. The fix rethrows the decode failure: scalacache.TypedApi._caching treats a throwing cache read as a miss, recomputes from the source block, and repopulates the key — the entry self-heals on the next call.

2. fix(routing) — invalidate and enable the MethodRouting lookup cache

The getMethodRoutings cache shipped disabled (TTL default 0) because nothing invalidated it. With it off, StarConnector re-queries the method_routing table on every intercepted connector call (up to twice per call), so a single API request issues dozens of identical queries.

  • createOrUpdateMethodRouting / deleteMethodRouting now pattern-delete *getMethodRoutings* — the memoize key embeds the literal method name, so one pattern delete clears all argument variants. Writes via /management/method_routings take effect immediately; the TTL is only a staleness backstop.
  • methodRouting.cache.ttl.seconds now defaults to 30, guarded to 0 in test/dev mode (same pattern as dynamicEntityTTL), so existing tests that mutate routings and assert immediately are unaffected.
  • Prop documented in sample.props.template.

Do not revert commit 1 independently of commit 2. The enabled MethodRouting cache relies on corrupt Redis entries healing as misses; reverting the deserialization fix alone would reintroduce TTL-long poisoned reads on a now-active cache.

Tests

  • New RedisDeserializeMissTest: undecodable bytes make the codec throw (never the legacy "NONE" sentinel), and a codec round-trip still works.
  • New MethodRoutingCacheInvalidationTest (cancelled via assume when no Redis is reachable): pattern invalidation forces a recompute on the next read; a corrupted entry behaves as a miss — exactly one recompute, key repopulated with valid bytes.
  • MethodRoutingTest (v3.1.0) and CacheEndpointsTest (v6.0.0) pass: 8 suites, 26 tests, 0 failed, 0 canceled (local Redis running).

Known follow-ups (out of scope here)

  • Duplicate getConnectorNameAndMethodRouting lookup per intercepted call (existing TODO in bankconnectors/package.scala) — the cache makes it cheap, dedup still worthwhile.
  • getEndpointMappings has the same disabled-TTL/no-invalidation pattern.
  • deleteKeysByPattern/scanKeys use Redis KEYS; should move to SCAN for large keyspaces.

hongwei1 added 3 commits July 7, 2026 11:39
Redis.deserialize returned the sentinel "NONE".asInstanceOf[T] when Kryo
failed to decode the cached bytes (corrupt entry, class-shape change across
a redeploy, Kryo registration drift). scalacache then treated that sentinel
as a cache HIT and handed a String to callers expecting the real type,
producing ClassCastException on every request until the TTL expired.

Rethrow the decode failure instead: scalacache.TypedApi._caching treats a
throwing cache read as a miss, recomputes the value from the source block,
and repopulates the key with a fresh, valid serialization - the cache
self-heals on the next call instead of staying poisoned.
The getMethodRoutings cache was shipped disabled (TTL default 0) because
nothing invalidated it: createOrUpdateMethodRouting/deleteMethodRouting
only touched the DB provider, so an enabled cache would serve stale
routings for a full TTL after every /management/method_routings write.
With the cache off, StarConnector re-queries the method_routing table on
every intercepted connector call - up to twice per call - so a single API
request issues dozens of identical getMethodRoutings queries.

- Invalidate on write: createOrUpdateMethodRouting and deleteMethodRouting
  now pattern-delete '*getMethodRoutings*'. The scalacache memoize key
  embeds the literal method name, so one pattern delete clears all
  argument variants.
- Enable the cache: methodRouting.cache.ttl.seconds now defaults to 30,
  guarded to 0 in test/dev mode (same pattern as dynamicEntityTTL) so
  tests that mutate routings and assert immediately are unaffected.
- Document the prop in sample.props.template.

The cache-invalidation and corrupt-entry self-healing behaviour is covered
by MethodRoutingCacheInvalidationTest (cancelled when no Redis is
reachable). This change depends on the previous commit: an enabled cache
is only safe when a corrupt Redis entry heals as a miss instead of
poisoning reads for the whole TTL.
@sonarqubecloud

Copy link
Copy Markdown

@simonredfern simonredfern merged commit 70612bc into OpenBankProject:develop Jul 13, 2026
12 checks passed
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