search

PromPHP / prometheus_client_php

Prometheus instrumentation library for PHP applications
https://prometheus.io/docs/concepts/metric_types/
Apache License 2.0
423 stars 93 forks source link

Redis storage uses KEYS for metrics collection #98

Open jaco opened 1 year ago

jaco commented 1 year ago

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 commented 1 year ago

I have deleted service monitor for test purposes :)

image
LKaemmerling commented 1 year ago

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 commented 1 year ago

@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).