馃挩 Expose Get keys from RedisCache #84
Conversation
|
P.S. I removed some internal informations from the description. If you need more information, please ping me. |
| * a value fetching. We don't want to hang forever if this is the case. | ||
| */ | ||
| this.emit('warn', 'Error while fetching keys from the Redis cache', error); | ||
| return []; |
There was a problem hiding this comment.
Could this an issue for the client? If an empty array is returned, how can we know if it's because there is no keys matching the pattern or if there are keys matching the patterns but we failed to return them?
There was a problem hiding this comment.
+1 to gr芒g's comment. What about changing this from returning a Promise<string[]> to returning a Promise<{ status: 'success' | 'timeout' | 'other-error', result: string[] }> ? That will let the client handle the timeout.
| @@ -27,6 +26,17 @@ export abstract class CacheInstance extends EventEmitter { | |||
| */ | |||
| public abstract getValue(key: string): Promise<CachableValue>; | |||
|
|
|||
| /** | |||
| * Find all keys matching the given pattern | |||
There was a problem hiding this comment.
Please document the O(n)-ness!
| * Find all keys matching the given pattern | |
| * Find all keys matching the given pattern. | |
| * | |
| * PERFORMANCE WARNING: this is O(keys) / will TRAVERSE ALL KEYS! | |
| * Both our Local & Redis implems can traverse a lot of keys quickly, | |
| * but be extremely careful calling this! This should be used for | |
| * occasional operations, not for high-frequency calls! |
| /** | ||
| * @inheritdoc | ||
| * | ||
| * Only support wildcard (*) globstyle pattern. Use RedisCache for more advanced pattern matching. |
There was a problem hiding this comment.
| * Only support wildcard (*) globstyle pattern. Use RedisCache for more advanced pattern matching. | |
| * Only supports wildcard (*) globstyle pattern. Use RedisCache for more advanced pattern matching. |
| * a value fetching. We don't want to hang forever if this is the case. | ||
| */ | ||
| this.emit('warn', 'Error while fetching keys from the Redis cache', error); | ||
| return []; |
There was a problem hiding this comment.
+1 to gr芒g's comment. What about changing this from returning a Promise<string[]> to returning a Promise<{ status: 'success' | 'timeout' | 'other-error', result: string[] }> ? That will let the client handle the timeout.
Problem
Having access to the list of keys is important in cases where we want to manipulate the cache such as deletes based on a pattern.
Solution
Exposing keys function from Redis allow us to find keys matching a certain pattern, thus allowing the dev to seek and delete expected caches.
Potential issue
Get Keys is O(n), n is the number of keys in cache and locks the instance while looping through all items. A potential solution is to use SCAN instead. But in this case, it will only be used in connectors which won't have a lot of cached items.