Refactor: extract Valkey Del command and ratelimit ring client injection - #4158
Draft
larry-dalmeida wants to merge 4 commits into
Draft
Refactor: extract Valkey Del command and ratelimit ring client injection#4158larry-dalmeida wants to merge 4 commits into
larry-dalmeida wants to merge 4 commits into
Conversation
…we can reuse the valkey ring client in the cache filter creation. Signed-off-by: Sandor Szücs <sandor.szuecs@zalando.de>
The len(results)==0 check was placed after the error-checking for-loop, making it read as dead code. Reorder to guard-first for clarity: validate non-empty results, then iterate to check for errors from the SET and EXPIRE commands, then return success. Signed-off-by: larry-dalmeida <hello@larrydalmeida.com>
Replace the EXPIRE key -1 workaround in ValkeyStorage.Delete() with a proper DEL command. This eliminates the 'hack' comment that szuecs flagged in comment r3501815470. Changes: - Add Del() method to valkeyRing (net/valkey.go) - Add Del() method to ValkeyRingClient (net/valkey.go) - Add Del() to the valkeyClient interface (filters/cache/valkey_storage.go) - Update Delete() to use Del instead of Expire key -1 (filters/cache/valkey_storage.go) - Add Del() to stubValkeyClient test stub (filters/cache/valkey_storage_test.go) This makes the code more idiomatic and removes the complexity of the EXPIRE key -1 semantics workaround. Signed-off-by: larry-dalmeida <hello@larrydalmeida.com>
larry-dalmeida
changed the title
The cache.NewCacheFilter block was carried over from the cherry-pick of f23acb2 but belongs exclusively in PR zalando#4033, which introduces the cache filter with full Options-struct wiring. This prereq only provides the shared valkeyRing creation and ratelimit registry injection. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Related Issue
Prerequisite PR for #4033, extracted to reduce the size of the cache filter PR.
Summary
net/valkey.go: addsDelmethod tovalkeyRingandValkeyRingClient; fixesSetWithExpireempty-result guard ordering (guard-before-loop for clarity)ratelimit/redis.go/ratelimit/valkey.go: exportsRedisMetricsPrefixandValkeyMetricsPrefixconstants so callers outside the package can reference themratelimit/registry.go: introducesNewRatelimitRegistryRedisandNewRatelimitRegistryValkey- injectable constructors that accept a pre-created ring client, enabling ring reuse across subsystemsskipper.go: creates the sharedvalkeyRingonce and passes it to both the ratelimit registry and (in feat: L2 Cache with Valkey Client Side Hash Ring #4033) the cache filter, replacing the oldNewSwarmRegistrycall