Make metrics collection optional/faster #330
Labels
API-breaking
This might introduce incompatible API changes
cpp-rust-driver-p2
Functionality required by cpp-rust-driver
enhancement
New feature or request
good first issue
Good for newcomers
Milestone
Comments
|
When tackling with this, let's consider cpp-rust-driver requirements wrt metrics. |
QuerthDP
added a commit
to QuerthDP/scylla-rust-driver
that referenced
this issue
Dec 9, 2024
I've added "metrics" crate feature which enables usage and gathering of metrics. Therefore everyone willing to use metrics in their code is required to add "metrics" feature in their Cargo.toml file or compile otherwise with --features metrics flag. This change was requested in scylladb#330
QuerthDP
added a commit
to QuerthDP/scylla-rust-driver
that referenced
this issue
Dec 10, 2024
I've added "metrics" crate feature which enables usage and gathering of metrics. Therefore everyone willing to use metrics in their code is required to add "metrics" feature in their Cargo.toml file or compile otherwise with --features metrics flag. This change was requested in scylladb#330
8 tasks
QuerthDP
added a commit
to QuerthDP/scylla-rust-driver
that referenced
this issue
Dec 11, 2024
I've added "metrics" crate feature which enables usage and gathering of metrics. Therefore everyone willing to use metrics in their code is required to add "metrics" feature in their Cargo.toml file or compile otherwise with --features metrics flag. This change was requested in scylladb#330
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Benchmarks done some time ago showed that the driver can spend a significant amount of time logging query latency (
scylla::transport::metrics::Metrics::log_query_latency11.68% on this flamegraph). It is probably due to the fact that logging latency to the metrics histogram can lead to congestion of a mutex guarding this histogram (logging takes place after each query completion).Mutex congestion problem could be fixed by using lock-free histogram, but unfortunately I can't find any crate providing one on crates.io. Another way to reduce this problem could be sharding histograms - making one per thread, in a similar way to this one. The last option sacrifices read performance, so it would be good to add some caching mechanism to
Metricsto avoid slowdowns caused by e.g. speculative execution policies asking for latency percentile for each query.I suppose we could also make metrics collection optional. It would be an obvious speedup for people who do not want to use metrics.
The text was updated successfully, but these errors were encountered: