Skip to content

cache: add redis key prefix support#8617

Open
cnaples79 wants to merge 2 commits intothanos-io:mainfrom
cnaples79:main
Open

cache: add redis key prefix support#8617
cnaples79 wants to merge 2 commits intothanos-io:mainfrom
cnaples79:main

Conversation

@cnaples79
Copy link
Copy Markdown

@cnaples79 cnaples79 commented Dec 31, 2025

Summary

  • add optional field to Redis cache config
  • prefix all redis keys in get/set operations when configured and return hits keyed by original keys
  • add a prefixed-config test case to cover the new behavior

Rationale

  • allows isolating cache keys when sharing a Redis instance or DB between components; defaults to empty string so existing setups are unaffected

Test Plan

  • unit tests updated (redis client tests include prefixed config)
  • all tests passing locally

Fixes #8608

Signed-off-by: Chase Naples <Cnaples79@gmail.com>
Signed-off-by: Chase Naples <Cnaples79@gmail.com>
Copy link
Copy Markdown
Member

@GiedriusS GiedriusS left a comment

Choose a reason for hiding this comment

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

But it's the intention to share results between instances so that they could re-use the results of computation. If the problem is that the keys&values are taking too much space, certain clusters or subset of services can use a different Redis cluster. Could you please elaborate on what problem you're trying to solve?

(Added a comment on the original issue, let's continue the discussion there).

start := time.Now()
results := make(map[string][]byte, len(keys))

prefixedKeys := make([]string, len(keys))
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Could we please add two separate paths - with and without a prefix? Otherwise, this very hot path will get 2 extra allocations even if one isn't using a prefix.

@GiedriusS
Copy link
Copy Markdown
Member

@cnaples79 ping, just one nit and we can merge this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add Prefix to Redis Cache

2 participants