prometheus_client_php icon indicating copy to clipboard operation
prometheus_client_php copied to clipboard

Redis storage uses KEYS for metrics collection

Open jaco opened this issue 3 years ago • 3 comments

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 avatar Nov 19 '22 11:11 jaco

I have deleted service monitor for test purposes :)

image

jaco avatar Nov 19 '22 12:11 jaco

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 avatar Nov 27 '22 21:11 LKaemmerling

@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: https://github.com/PromPHP/prometheus_client_php/pull/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).

LKaemmerling avatar Nov 28 '22 17:11 LKaemmerling