From 9658f095217d0d02b465682a7f0f4a40d858a1fa Mon Sep 17 00:00:00 2001
From: Lukasz Raczylo <lukasz@raczylo.com>
Date: Wed, 28 Oct 2026 22:19:10 +0000
Subject: [PATCH] fix(cache): count keys with SCAN not KEYS

CountQueries and CountQueriesWithPattern ran KEYS. With an empty
prefix, KEYS * builds a reply of every key on the server. The stats
endpoint calls it on each request. On a shared Dragonfly with about
1M keys, this raised memory on the server.

Count with a cursor SCAN instead. The count does not hold the key
list in memory. A test checks that no KEYS command is sent.
---
 cache/redis/redis.go                 | 34 +++++++++---
 cache/redis/redis_count_scan_test.go | 82 ++++++++++++++++++++++++++++
 2 files changed, 108 insertions(+), 8 deletions(-)
 create mode 100644 cache/redis/redis_count_scan_test.go

diff --git a/cache/redis/redis.go b/cache/redis/redis.go
index ec8996b..9fcc24a 100644
--- a/cache/redis/redis.go
+++ b/cache/redis/redis.go
@@ -32,6 +32,9 @@ func escapeGlobPattern(s string) string {
 	return globEscaper.Replace(s)
 }
 
+// scanCountBatch is the SCAN COUNT hint used when counting keys.
+const scanCountBatch = 1000
+
 type RedisConfig struct {
 	ctx         context.Context
 	client      *redis.Client
@@ -163,12 +166,28 @@ func (c *RedisConfig) Clear() error {
 // literally instead of being interpreted as a wildcard, which would
 // otherwise undercount or overcount keys in admin-dashboard stats.
 func (c *RedisConfig) CountQueries() (int64, error) {
-	pattern := escapeGlobPattern(c.prefix) + "*"
-	keys, err := c.client.Keys(c.ctx, pattern).Result()
-	if err != nil {
-		return 0, err
+	return c.scanCount(escapeGlobPattern(c.prefix) + "*")
+}
+
+// scanCount counts keys matching pattern with a cursor SCAN. KEYS builds the
+// whole reply in memory on the server and blocks it, which spiked a shared
+// Dragonfly to its memory limit when the prefix is empty.
+func (c *RedisConfig) scanCount(pattern string) (int64, error) {
+	var (
+		cursor uint64
+		total  int64
+	)
+	for {
+		keys, next, err := c.client.Scan(c.ctx, cursor, pattern, scanCountBatch).Result()
+		if err != nil {
+			return 0, fmt.Errorf("redis: scan at cursor %d: %w", cursor, err)
+		}
+		total += int64(len(keys))
+		cursor = next
+		if cursor == 0 {
+			return total, nil
+		}
 	}
-	return int64(len(keys)), nil
 }
 
 // CountQueriesWithPattern returns the number of keys owned by this cache
@@ -177,12 +196,11 @@ func (c *RedisConfig) CountQueries() (int64, error) {
 // itself is left as given so a caller can still use glob metacharacters
 // intentionally in the suffix it supplies.
 func (c *RedisConfig) CountQueriesWithPattern(pattern string) (int, error) {
-	fullPattern := escapeGlobPattern(c.prefix) + pattern
-	keys, err := c.client.Keys(c.ctx, fullPattern).Result()
+	total, err := c.scanCount(escapeGlobPattern(c.prefix) + pattern)
 	if err != nil {
 		return 0, err
 	}
-	return len(keys), nil
+	return int(total), nil
 }
 
 // GetMemoryUsage returns an approximation of memory usage for Redis.
diff --git a/cache/redis/redis_count_scan_test.go b/cache/redis/redis_count_scan_test.go
new file mode 100644
index 0000000..8a85a0d
--- /dev/null
+++ b/cache/redis/redis_count_scan_test.go
@@ -0,0 +1,82 @@
+package libpack_cache_redis
+
+import (
+	"context"
+	"fmt"
+	"net"
+	"strings"
+	"sync"
+	"testing"
+
+	"github.com/alicebob/miniredis/v2"
+	redis "github.com/redis/go-redis/v9"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+type commandRecorder struct {
+	mu   sync.Mutex
+	seen []string
+}
+
+func (r *commandRecorder) DialHook(next redis.DialHook) redis.DialHook {
+	return func(ctx context.Context, network, addr string) (net.Conn, error) {
+		return next(ctx, network, addr)
+	}
+}
+
+func (r *commandRecorder) ProcessHook(next redis.ProcessHook) redis.ProcessHook {
+	return func(ctx context.Context, cmd redis.Cmder) error {
+		r.mu.Lock()
+		r.seen = append(r.seen, strings.ToLower(cmd.Name()))
+		r.mu.Unlock()
+		return next(ctx, cmd)
+	}
+}
+
+func (r *commandRecorder) ProcessPipelineHook(next redis.ProcessPipelineHook) redis.ProcessPipelineHook {
+	return next
+}
+
+func (r *commandRecorder) count(name string) int {
+	r.mu.Lock()
+	defer r.mu.Unlock()
+	n := 0
+	for _, s := range r.seen {
+		if s == name {
+			n++
+		}
+	}
+	return n
+}
+
+// TestCount_UsesScanNotKeys guards against reintroducing KEYS, which builds
+// the whole key list server-side and spiked a shared Dragonfly to its limit.
+func TestCount_UsesScanNotKeys(t *testing.T) {
+	t.Parallel()
+	s, err := miniredis.Run()
+	require.NoError(t, err)
+	t.Cleanup(s.Close)
+
+	rc, err := New(&RedisClientConfig{RedisServer: s.Addr()})
+	require.NoError(t, err)
+
+	const total = 2500
+	for i := 0; i < total; i++ {
+		require.NoError(t, rc.Set(fmt.Sprintf("k%d", i), []byte("v"), 0))
+	}
+
+	rec := &commandRecorder{}
+	rc.client.AddHook(rec)
+
+	count, err := rc.CountQueries()
+	require.NoError(t, err)
+	assert.Equal(t, int64(total), count)
+
+	patternCount, err := rc.CountQueriesWithPattern("k1*")
+	require.NoError(t, err)
+	assert.Equal(t, 1111, patternCount, "k1, k10-k19, k100-k199, k1000-k1999")
+
+	assert.Zero(t, rec.count("keys"), "counting must not issue KEYS")
+	assert.GreaterOrEqual(t, rec.count("scan"), 2, "counting must issue SCAN")
+}