Skip to content

Commit e31d73b

Browse files
committed
fix(keyvalue): treat undecodable cached values as a cache miss
A GetWithValue deserialization error in RememberFor was returned as fatal. On a Redis upgrade the metrics counters live under the same keys as before but were stored as plain int64, so the first decode into the new envelope would fail and the metric would break permanently. Treat such errors as a miss and recompute/overwrite so the cache self-heals.
1 parent 9a810f7 commit e31d73b

2 files changed

Lines changed: 30 additions & 5 deletions

File tree

pkg/modules/keyvalue/keyvalue.go

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -155,14 +155,14 @@ type expiringValue[T any] struct {
155155
// older than ttl. On a miss or once expired, it executes fn, caches the result for
156156
// ttl and returns it. If fn returns an error, nothing is cached.
157157
// T must be a concrete (non-pointer) type.
158+
//
159+
// A value that cannot be deserialized into the expected type is treated as a cache
160+
// miss and overwritten, so the cache self-heals across upgrades that change what a key
161+
// stores (e.g. a key that previously held a plain int64 in Redis).
158162
func RememberFor[T any](key string, ttl time.Duration, fn func() (T, error)) (T, error) {
159163
var cached expiringValue[T]
160164
exists, err := GetWithValue(key, &cached)
161-
if err != nil {
162-
var zero T
163-
return zero, err
164-
}
165-
if exists && time.Now().Before(cached.ExpiresAt) {
165+
if err == nil && exists && time.Now().Before(cached.ExpiresAt) {
166166
return cached.Value, nil
167167
}
168168

pkg/modules/keyvalue/keyvalue_test.go

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -132,3 +132,28 @@ func TestRememberForErrorDoesNotStore(t *testing.T) {
132132
require.NoError(t, err2)
133133
assert.False(t, exists)
134134
}
135+
136+
// getWithValueErrorStore simulates a backend that cannot deserialize an existing value
137+
// into the requested type, e.g. a key that held a plain int64 before the cache started
138+
// storing a struct (the pre-refactor metrics counters in Redis).
139+
type getWithValueErrorStore struct {
140+
*memory.Storage
141+
}
142+
143+
func (s *getWithValueErrorStore) GetWithValue(string, interface{}) (bool, error) {
144+
return false, errors.New("decode error")
145+
}
146+
147+
func TestRememberForRecomputesWhenStoredValueCannotBeDeserialized(t *testing.T) {
148+
store = &getWithValueErrorStore{memory.NewStorage()}
149+
150+
called := 0
151+
val, err := RememberFor("foo", time.Hour, func() (int64, error) {
152+
called++
153+
return 42, nil
154+
})
155+
156+
require.NoError(t, err)
157+
assert.Equal(t, int64(42), val)
158+
assert.Equal(t, 1, called)
159+
}

0 commit comments

Comments
 (0)