Duplicated namespace in redis keys and issues when using a single store for different caches.

leo-fresha commented 1 month ago

Bug description

Every newly instantiated Keyv cache sets the namespace attribute of the underlying store, which has two implications (at least when using the KeyvRedis backend):

the namespace is duplicated. Redis keys have names like sets:namespace:<namespace>:<namespace>:<key> instead of sets:namespace:<namespace>:<key> (I wonder if we need such a long prefix at all, vs just <prefix>:<namespace>:<key>, where <prefix> can be configured by the user when instantianing the store, but that's a different story...)
it's not possible to safely use the same store for multiple caches, as instantiating a new cache modifies the namespace of the underlying store, which is used to retrieve keys, thus breaking previous stored entries.

How to reproduce

Given this snippet which instantiates two Keyv caches which use the same store (which, in turn, share the same pre-existing Redis connection).

import KeyvRedis from '@keyv/redis';
import Redis from 'ioredis';
import Keyv from 'keyv';

const redisClient = new Redis(process.env.REDIS_URI);
const redisStore = new KeyvRedis(redisClient, { useRedisSets: false });

const KEY = 'jane';

const addressCache = new Keyv({
  store: redisStore,
  namespace: 'addresses',
  ttl: 24 * 60 * 60, // 1 day
});
await addressCache.set(KEY, 'Somewhere in London');

const professionCache = new Keyv({
  store: redisStore,
  namespace: 'professions',
  ttl: 10 * 24 * 60 * 60, // 10 days
});
await professionCache.set(KEY, 'Doctor');

console.log(await addressCache.get(KEY));
console.log(await professionCache.get(KEY));

await redisClient.disconnect();

Running the script you'll get

undefined
'Doctor'

Instead of the expected:

'Somewhere in London'
'Doctor'

And this is the output of redis MONITOR command while running the script:

1727813797.264412 [0 172.23.0.1:52014] "info"
1727813797.267410 [0 172.23.0.1:52014] "set" "sets:namespace:addresses:addresses:jane" "{\"value\":\"Somewhere in London\",\"expires\":1727813883685}" "PX" "86400"
1727813797.270006 [0 172.23.0.1:52014] "set" "sets:namespace:professions:professions:jane" "{\"value\":\"Doctor\",\"expires\":1727814661298}" "PX" "864000"
1727813797.271760 [0 172.23.0.1:52014] "get" "sets:namespace:professions:addresses:jane"
1727813797.275019 [0 172.23.0.1:52014] "get" "sets:namespace:professions:professions:jane"

You can see the duplicated namespace and the full key for the address cache entries changing after the professions cache is instantiated.

Suggested resolution

Keyv should not set the namespace of the underlying store at all. Store namespace (or, better, prefix) should be optional and configurable by the user at the store level. This would result in much cleaner redis keys like <store-prefix>:<key cache namespace>:<key>. It would also allow users to configure short prefixes/namespaces to save memory, which can be quite important for redis and other stores where key size is consequential.

jaredwray commented 1 month ago

@leo-fresha thanks for posting this and we are working on an update around this to make it better.

jaredwray commented 3 weeks ago

@leo-fresha - we are working on the rewrite now. I am going to close this issue and add it as an item to resolve on the main thread.

jaredwray / keyv