mirror of
https://github.com/lukaszraczylo/graphql-monitoring-proxy.git
synced 2026-10-29 07:18:06 +00:00
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.
This commit is contained in:
Vendored
+26
-8
@@ -32,6 +32,9 @@ func escapeGlobPattern(s string) string {
|
|||||||
return globEscaper.Replace(s)
|
return globEscaper.Replace(s)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// scanCountBatch is the SCAN COUNT hint used when counting keys.
|
||||||
|
const scanCountBatch = 1000
|
||||||
|
|
||||||
type RedisConfig struct {
|
type RedisConfig struct {
|
||||||
ctx context.Context
|
ctx context.Context
|
||||||
client *redis.Client
|
client *redis.Client
|
||||||
@@ -163,12 +166,28 @@ func (c *RedisConfig) Clear() error {
|
|||||||
// literally instead of being interpreted as a wildcard, which would
|
// literally instead of being interpreted as a wildcard, which would
|
||||||
// otherwise undercount or overcount keys in admin-dashboard stats.
|
// otherwise undercount or overcount keys in admin-dashboard stats.
|
||||||
func (c *RedisConfig) CountQueries() (int64, error) {
|
func (c *RedisConfig) CountQueries() (int64, error) {
|
||||||
pattern := escapeGlobPattern(c.prefix) + "*"
|
return c.scanCount(escapeGlobPattern(c.prefix) + "*")
|
||||||
keys, err := c.client.Keys(c.ctx, pattern).Result()
|
}
|
||||||
if err != nil {
|
|
||||||
return 0, err
|
// 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
|
// 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
|
// itself is left as given so a caller can still use glob metacharacters
|
||||||
// intentionally in the suffix it supplies.
|
// intentionally in the suffix it supplies.
|
||||||
func (c *RedisConfig) CountQueriesWithPattern(pattern string) (int, error) {
|
func (c *RedisConfig) CountQueriesWithPattern(pattern string) (int, error) {
|
||||||
fullPattern := escapeGlobPattern(c.prefix) + pattern
|
total, err := c.scanCount(escapeGlobPattern(c.prefix) + pattern)
|
||||||
keys, err := c.client.Keys(c.ctx, fullPattern).Result()
|
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return 0, err
|
return 0, err
|
||||||
}
|
}
|
||||||
return len(keys), nil
|
return int(total), nil
|
||||||
}
|
}
|
||||||
|
|
||||||
// GetMemoryUsage returns an approximation of memory usage for Redis.
|
// GetMemoryUsage returns an approximation of memory usage for Redis.
|
||||||
|
|||||||
Vendored
+82
@@ -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")
|
||||||
|
}
|
||||||
Reference in New Issue
Block a user