seastar
seastar copied to clipboard
metrics: change value_vector type to std::deque
We've run into an oversized allocation (over 1MB) coming from the metrics subsystem. A snippet of the backtrace is seen below:
(inlined by) std::__1::__split_buffer<seastar::metrics::impl::metric_value, std::__1::allocator<seastar::metrics::impl::metric_value>&>::__split_buffer(unsigned long, unsigned long, std::__1::allocator<seastar::metrics::impl::metric_value>&) at /vectorized/llvm/bin/../include/c++/v1/__split_buffer:323
(inlined by) std::__1::vector<seastar::metrics::impl::metric_value, std::__1::allocator<seastar::metrics::impl::metric_value>>::reserve(unsigned long) at /vectorized/llvm/bin/../include/c++/v1/vector:1503
(inlined by) seastar::metrics::impl::get_values(int) at /v/build/v_deps_build/seastar-prefix/src/seastar/src/core/metrics.cc:422
auto seastar::prometheus::get_map_value(seastar::prometheus::metrics_families_per_shard&, int)::$_0::operator()<unsigned int>(unsigned int) const::'lambda'()::operator()() const at /v/build/v_deps_build/seastar-prefix/src/seastar/src/core/prometheus.cc:224
The vector being reserved in this case is the value_vector
[1].
In a previous change[2] _current_metrics
(returned by get_local_impl()->functions()
) had its inner container swapped out for std::deque
because we'd previously seen the container grow large enough to see oversized allocations there. However, get_values()
reserves a value_vector
of the same number of elements as each std::deque
, so a similar problem remains.
Since these containers are going to have the same cardinality, this patch updates the value_vector
to also be a std::deque
.
[1] https://github.com/scylladb/seastar/blob/b73f5ffa9e1eb13e9f2c5afde58f70c0df6efc73/src/core/metrics.cc#L370-L371 [2] https://github.com/scylladb/seastar/commit/c4b1f70162652075ef43871824350aa4fc4893fe
@amnonh Hello! Gentle nudge about this when you get the chance.
I'm happy to answer questions and provide additional context if needed. Alternatively, I could tag the reviewers from https://github.com/scylladb/seastar/pull/1741 as they may also have some context -- whatever is more convenient for you all.
LGTM
Thank you @amnonh! What is the protocol for getting this merged?