fix(redis): deduplicate keys returned by SCAN - #199
Merged
Merged
Conversation
SCAN may return a key more than once when the keyspace shrinks during the iteration, and get_all_keys() passed the duplicates through to CacheManager and the monitoring routes. Closes #173
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
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.
Closes #173.
Problem
SCAN guarantees that every key present for the whole iteration is returned, but not that it is returned only once. When the keyspace shrinks mid-scan, a key can come back twice.
_scan_keyspassed duplicates through toget_all_keys(), and so toCacheManager, the monitoring routes and session enumeration.Reproduced on Redis (
redis:alpine):p:*keys and 60 000 other keys, and startSCAN MATCH p:* COUNT 2000.UNLINKthe other keys and continue the scan.Change
_scan_keyscollects keys into adict, which deduplicates them while keeping the order in which each key was first seen. Every caller benefits:get_all_keys,get_cache_data, and theclear*methods, which also stop sending a key twice toDEL.Tests
test_redis_scan_results_are_deduplicatedwrapsclient.scanso that each later page repeats a key already returned. It spans three SCAN pages.CACHEX_REQUIRE_LIVE_SERVERS=1): 852 passed, 100% coverage.