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

Redis storage uses KEYS for metrics collection #98

Open
jaco opened this issue Nov 19, 2022 · 3 comments
Open

Redis storage uses KEYS for metrics collection #98

jaco opened this issue Nov 19, 2022 · 3 comments

Comments

@jaco
Copy link

jaco commented Nov 19, 2022

KEYS is very expensive command, according to manual:

Warning: consider KEYS as a command that should only be used in production environments with extreme care. It 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 in your regular application code. If you're looking for a way to find keys in a subset of your keyspace, consider using SCAN or sets.

I saw that there was an attempt to change KEYS to SCAN and performance dropped.

IMO redis storage should use some kind of list/set to keep track of used metrics.

In our deployment, keys command its responsible for 90%+ load of whole redis instance and we use redis storage ONLY for jobs (APCu for http requests).

@jaco
Copy link
Author

jaco commented Nov 19, 2022

I have deleted service monitor for test purposes :)

image

@LKaemmerling
Copy link
Member

Hey @jaco,

i guess solving this will require a breaking change, as we would need to store something like metadata. At the moment I would like to avoid making a breaking change. Maybe we can go a similar route like we did it with APC (APCNg). I guess this would be the best idea for now, or do you maybe have a better idea?

@LKaemmerling
Copy link
Member

LKaemmerling commented Nov 28, 2022

@jaco I have a working "RedisNG" ready. The tests pass locally but for the CI I still fight against them. You can find it in this MR: #99

Would it be possible that you test it against your workload? It is a completely new storage engine as I changed the way how the summary keys are saved (this was basically the only part that used KEYS).

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

No branches or pull requests

2 participants