From ac0fcfbe6a45e7e747f50dc7b08d4b36c9b17218 Mon Sep 17 00:00:00 2001 From: Lukasz Raczylo Date: Tue, 3 Nov 2026 00:42:17 +0000 Subject: [PATCH] fix(redis): never send a zero or shortened expiry Set and SetMany sent PSETEX with 0 for a TTL under 1ms, which Redis rejects. They sent SETEX with truncated seconds for a fractional TTL, so keys expired early. A shared helper now rounds up to a millisecond and uses SETEX only for whole seconds. --- internal/cache/backends/redis.go | 38 ++++++++-------- .../cache/backends/redis_ttl_bounds_test.go | 45 +++++++++++++++++++ 2 files changed, 65 insertions(+), 18 deletions(-) create mode 100644 internal/cache/backends/redis_ttl_bounds_test.go diff --git a/internal/cache/backends/redis.go b/internal/cache/backends/redis.go index 96ca423..69b4992 100644 --- a/internal/cache/backends/redis.go +++ b/internal/cache/backends/redis.go @@ -120,15 +120,8 @@ func (r *RedisBackend) Set(ctx context.Context, key string, value []byte, ttl ti // Use PSETEX for millisecond precision, SETEX for second precision if ttl > 0 { - ttlMillis := ttl.Milliseconds() - if ttlMillis < 1000 { - // Use PSETEX for sub-second TTLs (millisecond precision) - _, err = conn.Do("PSETEX", prefixedKey, fmt.Sprintf("%d", ttlMillis), string(value)) - } else { - // Use SETEX for larger TTLs (second precision) - ttlSeconds := int(ttl.Seconds()) - _, err = conn.Do("SETEX", prefixedKey, fmt.Sprintf("%d", ttlSeconds), string(value)) - } + cmd, arg := ttlCommand(ttl) + _, err = conn.Do(cmd, prefixedKey, arg, string(value)) } else { _, err = conn.Do("SET", prefixedKey, string(value)) } @@ -624,6 +617,19 @@ func isRetryableError(err error) bool { return false } +// ttlCommand picks SETEX for whole seconds and PSETEX otherwise. The expiry is +// rounded up to a millisecond and is never zero, so a key never dies early. +func ttlCommand(ttl time.Duration) (cmd, arg string) { + ms := (ttl + time.Millisecond - 1).Milliseconds() + if ms < 1 { + ms = 1 + } + if ms >= 1000 && ms%1000 == 0 { + return "SETEX", fmt.Sprintf("%d", ms/1000) + } + return "PSETEX", fmt.Sprintf("%d", ms) +} + // SetMany stores multiple values in Redis using pipelining for efficiency // This reduces N round-trips to a single round-trip func (r *RedisBackend) SetMany(ctx context.Context, items map[string][]byte, ttl time.Duration) error { @@ -658,20 +664,16 @@ func (r *RedisBackend) SetMany(ctx context.Context, items map[string][]byte, ttl pipeline := conn.NewPipeline() // Queue all SET commands - ttlSeconds := int(ttl.Seconds()) - ttlMillis := ttl.Milliseconds() + var ttlCmd, ttlArg string + if ttl > 0 { + ttlCmd, ttlArg = ttlCommand(ttl) + } for key, value := range items { prefixedKey := r.prefixKey(key) if ttl > 0 { - if ttlMillis < 1000 { - // Use PSETEX for sub-second TTLs - pipeline.Queue("PSETEX", prefixedKey, fmt.Sprintf("%d", ttlMillis), string(value)) - } else { - // Use SETEX for larger TTLs - pipeline.Queue("SETEX", prefixedKey, fmt.Sprintf("%d", ttlSeconds), string(value)) - } + pipeline.Queue(ttlCmd, prefixedKey, ttlArg, string(value)) } else { pipeline.Queue("SET", prefixedKey, string(value)) } diff --git a/internal/cache/backends/redis_ttl_bounds_test.go b/internal/cache/backends/redis_ttl_bounds_test.go new file mode 100644 index 0000000..0ab3ca7 --- /dev/null +++ b/internal/cache/backends/redis_ttl_bounds_test.go @@ -0,0 +1,45 @@ +package backends + +import ( + "context" + "testing" + "time" + + "github.com/stretchr/testify/require" +) + +func TestRedisSetTTLNeverZeroOrEarly(t *testing.T) { + mr := NewMiniredisServer(t) + b, err := NewRedisBackend(DefaultRedisConfig(mr.GetAddr())) + require.NoError(t, err) + defer b.Close() + ctx := context.Background() + + t.Run("sub-millisecond ttl is accepted", func(t *testing.T) { + require.NoError(t, b.Set(ctx, "sub", []byte("v"), 500*time.Microsecond)) + }) + t.Run("sub-millisecond ttl accepted by SetMany", func(t *testing.T) { + require.NoError(t, b.SetMany(ctx, map[string][]byte{"sub-many": []byte("v")}, 500*time.Microsecond)) + }) + t.Run("fractional seconds are not rounded down", func(t *testing.T) { + require.NoError(t, b.Set(ctx, "frac", []byte("v"), 1999*time.Millisecond)) + _, ttl, ok, err := b.Get(ctx, "frac") + require.NoError(t, err) + require.True(t, ok) + require.Greater(t, ttl, time.Second, "1999ms ttl must not be stored as 1s") + }) + t.Run("fractional seconds SetMany", func(t *testing.T) { + require.NoError(t, b.SetMany(ctx, map[string][]byte{"frac-many": []byte("v")}, 1999*time.Millisecond)) + _, ttl, ok, err := b.Get(ctx, "frac-many") + require.NoError(t, err) + require.True(t, ok) + require.Greater(t, ttl, time.Second) + }) + t.Run("whole seconds still work", func(t *testing.T) { + require.NoError(t, b.Set(ctx, "whole", []byte("v"), 5*time.Second)) + _, ttl, ok, err := b.Get(ctx, "whole") + require.NoError(t, err) + require.True(t, ok) + require.Greater(t, ttl, 3*time.Second) + }) +}