Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: replace redis keys call with scan #106

Merged
merged 1 commit into from
Sep 5, 2023

Conversation

szesch
Copy link
Contributor

@szesch szesch commented Sep 4, 2023

We've started seeing entries in the Redis slowlog related to retrieving keys for kong_acme:renew_config. Here is a sample:

 1) 1) (integer) 484
    2) (integer) 1693836071
    3) (integer) 14540
    4) 1) "keys"
       2) "kong_acme:renew_config:*"
    5) "<redacted>"
    6) ""
 2) 1) (integer) 483
    2) (integer) 1693833074
    3) (integer) 15386
    4) 1) "keys"
       2) "kong_acme:renew_config:*"
    5) "<redacted>"
    6) ""
 3) 1) (integer) 482
    2) (integer) 1693833067
    3) (integer) 15158
    4) 1) "keys"
       2) "kong_acme:renew_config:*"
    5) "<redacted>"
    6) ""

This PR replaces the usage of keys with scan. Per Redis documentation scan is preferred over keys in production environments.

Warning: consider [KEYS](https://redis.io/commands/keys) as a command that should only be used in production environments with extreme care.

[KEYS](https://redis.io/commands/keys) may ruin performance when it is executed against large databases. This
command is intended for debugging and special operations, such as changing your keyspace layout. Don't use [KEYS]
https://redis.io/commands/keys) in your regular application code. If you're looking for a way to find keys in a subset of
your keyspace, consider using [SCAN](https://redis.io/commands/scan) or [sets](https://redis.io/topics/data-types#sets).

A new config option scan_count is introduced to configure how many keys should be returned each call. It defaults to 10 which is the Redis default. Note that Redis does not guarantee that it will obey this value.

An additional test was also added with 50 keys that tests multiple scan calls.

@fffonion fffonion merged commit d806c19 into fffonion:master Sep 5, 2023
5 checks passed
@fffonion
Copy link
Owner

fffonion commented Sep 5, 2023

Thank you @szesch !

@szesch szesch deleted the redis-use-scan branch September 5, 2023 15:04
fffonion pushed a commit to Kong/kong that referenced this pull request Sep 26, 2023
SRE has started seeing entries in the Redis slowlog related to retrieving keys for kong_acme:renew_config. lua-resty-acme was recently changed to use Redis scan instead of keys for performance reasons. See the lua-resty-acme PR for more details fffonion/lua-resty-acme#106.

This PR exposes a new configuration field for Redis storage that controls how many keys are returned in a scan call and bumps lua-resty-acme to 0.12.0.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants