dotCMS / dotCMS/core

Lettuce Redis cache: cross-cluster cache wipe, missing/blank cache-stats region, and related defects

Open
#36,035 1 comment 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Team : Falcon Type : Defect
Dominant language
Java
Stars
970
Forks
486
Avg merge
3d 33m
Merged PRs (30d)
170

Description

I had RedisCache set up by default in the temu cluster and found:

  • there was no cache reporting in the backend
  • A full cache flush did not work - this is because it was actually trying to run a full flushall on redis without prefixing which our per-tenant namespaces did not allow. Yea! Redis permissions and isolation worked!

I asked Claudio to fix (and simplify all the unnecessary abstractions) and here is what we got:


Problem

A deep review of the com.dotcms.cache.lettuce package surfaced several defects in the Lettuce-based Redis cache (RedisCache / MasterReplicaLettuceClient). The most serious break the package's own stated design goal (per-cluster key isolation in a shared Redis space) and the cache-stats maintenance screen.

Defects
  1. Cross-cluster cache wipe (HIGH). RedisCache.remove(group) and removeAll() build *PREFIX* match patterns without the cluster id. In a shared Redis space (which the clusterId prefix exists specifically to support), these patterns match and delete other clusters'/tenants' keys. keyCount/scanKeys are correctly cluster-scoped, so the codebase is internally inconsistent.

  2. deleteFromPattern is fragile (HIGH). It runs redis.call('del', unpack(redis.call('keys', '<pattern>'))) via eval:

    • errors out (wrong number of arguments for 'del') when the pattern matches nothing — and the error is swallowed (async, result discarded), so the delete silently no-ops;
    • uses blocking KEYS (O(N), blocks the Redis event loop);
    • is vulnerable to Lua string injection via the interpolated pattern.
  3. Redis region missing / blank on the cache-stats screen (HIGH). RedisCache.getStats() / keyCount() can throw on a null pooled connection (conn.isOpen() on a null borrow). CacheProviderAPIImpl.getStats() drops any provider whose getStats() throws, so the Redis region disappears from the cache-stats maintenance screen entirely. When the group set is empty it also produced an empty/blank table (no fallback row like CaffineCache has).

  4. delete(K...) never matches stored keys (MED). The varargs delete converts keys with keyToStringConverter instead of wrapKey, so it omits the cluster prefix and never matches what was stored (contrast the correct single-key delete and deleteNonBlocking).

  5. Pub/sub connection leak + non-thread-safe subscriber lists (MED). The pub/sub connection is leaked when it cannot be opened; subscriber lists are plain ArrayLists mutated/iterated across concurrent subscribe/unsubscribe.

  6. Misc: DotObjectCodec.decodeValue uses bytes.array() (ignores buffer position/limit/offset); unwrapKey/getKeys use a global String.replace instead of a leading-anchored strip; sync set(k,v,ttl) doesn't guard ttl == -1; RedisClientFactory swallows reflection failures and uses deprecated newInstance(); duplicate/dead code (removeKeysRaw, dead getStats INFO block).

Scope / constraint

The RedisClient interface is kept intact (generics + all methods); genuinely-unused methods are @Deprecated rather than removed.

Notes

  • After the fix, remove(group) / removeAll() become synchronous (SCAN + UNLINK) instead of async fire-and-forget, matching the Caffeine/H22 providers. The one production caller is RedisStoragePersistenceAPI.remove(...).

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with the com.dotcms.cache.lettuce package, especially RedisCache and MasterReplicaLettuceClient, then inspect CacheProviderAPIImpl and RedisStoragePersistenceAPI.remove. Trace the cluster prefix, deletion, stats, connection, and subscriber paths described in the issue. Done means isolated synchronous cache wipes, reliable cache statistics, corrected key handling, and no identified connection or decoding defects while keeping RedisClient intact.

Written by the indexing model from the issue text.

Assessment

Tech stack
java, redis
Domain
backend, databases
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Quiet
Clarity
Mostly clear
Newbie friendliness
42/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.