From c5a136c3b563ab04703232845cdfbbc6c841aeeb Mon Sep 17 00:00:00 2001 From: Asias He Date: Wed, 18 Jun 2025 19:31:32 +0800 Subject: [PATCH] storage_service: Use utils::chunked_vector to avoid big allocation The following was seen: ``` !WARNING | scylla[6057]: [shard 12:strm] seastar_memory - oversized allocation: 212992 bytes. This is non-fatal, but could lead to latency and/or fragmentation issues. Please report: at [Backtrace #0] void seastar::backtrace(seastar::current_backtrace_tasklocal()::$_0&&, bool) at ./build/release/seastar/./seastar/include/seastar/util/backtrace.hh:89 (inlined by) seastar::current_backtrace_tasklocal() at ./build/release/seastar/./build/release/seastar/./seastar/src/util/backtrace.cc:99 seastar::current_tasktrace() at ./build/release/seastar/./build/release/seastar/./seastar/src/util/backtrace.cc:136 seastar::current_backtrace() at ./build/release/seastar/./build/release/seastar/./seastar/src/util/backtrace.cc:169 seastar::memory::cpu_pages::warn_large_allocation(unsigned long) at ./build/release/seastar/./build/release/seastar/./seastar/src/core/memory.cc:848 seastar::memory::allocate_slowpath(unsigned long) at ./build/release/seastar/./build/release/seastar/./seastar/src/core/memory.cc:911 operator new(unsigned long) at ./build/release/seastar/./build/release/seastar/./seastar/src/core/memory.cc:1706 std::allocator::allocate(unsigned long) at /usr/lib/gcc/x86_64-redhat-linux/14/../../../../include/c++/14/bits/allocator.h:196 (inlined by) std::allocator_traits >::allocate(std::allocator&, unsigned long) at /usr/lib/gcc/x86_64-redhat-linux/14/../../../../include/c++/14/bits/alloc_traits.h:515 (inlined by) std::_Vector_base >::_M_allocate(unsigned long) at /usr/lib/gcc/x86_64-redhat-linux/14/../../../../include/c++/14/bits/stl_vector.h:380 (inlined by) void std::vector >::_M_realloc_append(dht::token_range_endpoints const&) at /usr/lib/gcc/x86_64-redhat-linux/14/../../../../include/c++/14/bits/vector.tcc:596 locator::describe_ring(replica::database const&, gms::gossiper const&, seastar::basic_sstring const&, bool) at /usr/lib/gcc/x86_64-redhat-linux/14/../../../../include/c++/14/bits/stl_vector.h:1294 std::__n4861::coroutine_handle > >::promise_type>::resume() const at /usr/lib/gcc/x86_64-redhat-linux/14/../../../../include/c++/14/coroutine:242 (inlined by) seastar::internal::coroutine_traits_base > >::promise_type::run_and_dispose() at ././seastar/include/seastar/core/coroutine.hh:80 seastar::reactor::do_run() at ./build/release/seastar/./build/release/seastar/./seastar/src/core/reactor.cc:2635 std::_Function_handler::_M_invoke(std::_Any_data const&) at ./build/release/seastar/./build/release/seastar/./seastar/src/core/reactor.cc:4684 ``` Fix by using chunked_vector. Fixes #24158 Closes scylladb/scylladb#24561 --- db/virtual_tables.cc | 6 +++--- locator/util.cc | 4 ++-- locator/util.hh | 3 ++- service/storage_proxy.cc | 2 +- service/storage_proxy.hh | 2 +- service/storage_service.cc | 6 +++--- service/storage_service.hh | 4 ++-- 7 files changed, 14 insertions(+), 13 deletions(-) diff --git a/db/virtual_tables.cc b/db/virtual_tables.cc index 40dc637fc8..46a99b002c 100644 --- a/db/virtual_tables.cc +++ b/db/virtual_tables.cc @@ -165,7 +165,7 @@ public: }); } - future<> emit_ring(result_collector& result, const dht::decorated_key& dk, const sstring& table_name, std::vector ranges) { + future<> emit_ring(result_collector& result, const dht::decorated_key& dk, const sstring& table_name, utils::chunked_vector ranges) { co_await result.emit_partition_start(dk); std::ranges::sort(ranges, std::ranges::less(), std::mem_fn(&dht::token_range_endpoints::_start_token)); @@ -219,11 +219,11 @@ public: co_return; } const auto& table_name = table->schema()->cf_name(); - std::vector ranges = co_await _ss.describe_ring_for_table(e.name, table_name); + utils::chunked_vector ranges = co_await _ss.describe_ring_for_table(e.name, table_name); co_await emit_ring(result, e.key, table_name, std::move(ranges)); }); } else { - std::vector ranges = co_await _ss.describe_ring(e.name); + utils::chunked_vector ranges = co_await _ss.describe_ring(e.name); co_await emit_ring(result, e.key, "", std::move(ranges)); } } diff --git a/locator/util.cc b/locator/util.cc index dce81b7190..17474179d3 100644 --- a/locator/util.cc +++ b/locator/util.cc @@ -95,9 +95,9 @@ get_range_to_address_map(locator::effective_replication_map_ptr erm) { return get_range_to_address_map(erm, erm->get_token_metadata_ptr()->sorted_tokens()); } -future> +future> describe_ring(const replica::database& db, const gms::gossiper& gossiper, const sstring& keyspace, bool include_only_local_dc) { - std::vector ranges; + utils::chunked_vector ranges; auto erm = db.find_keyspace(keyspace).get_vnode_effective_replication_map(); std::unordered_map range_to_address_map = co_await ( diff --git a/locator/util.hh b/locator/util.hh index c66e400680..8f2f749392 100644 --- a/locator/util.hh +++ b/locator/util.hh @@ -12,6 +12,7 @@ #include "dht/i_partitioner_fwd.hh" #include "inet_address_vectors.hh" #include "locator/abstract_replication_strategy.hh" +#include "utils/chunked_vector.hh" namespace replica { class database; @@ -22,7 +23,7 @@ namespace gms { } namespace locator { - future> describe_ring(const replica::database& db, const gms::gossiper& gossiper, const sstring& keyspace, bool include_only_local_dc = false); + future> describe_ring(const replica::database& db, const gms::gossiper& gossiper, const sstring& keyspace, bool include_only_local_dc = false); future> get_range_to_address_map( locator::effective_replication_map_ptr erm, const std::vector& sorted_tokens); } \ No newline at end of file diff --git a/service/storage_proxy.cc b/service/storage_proxy.cc index 0bbbcef01c..044844cdbf 100644 --- a/service/storage_proxy.cc +++ b/service/storage_proxy.cc @@ -6965,7 +6965,7 @@ locator::token_metadata_ptr storage_proxy::get_token_metadata_ptr() const noexce return _shared_token_metadata.get(); } -future> storage_proxy::describe_ring(const sstring& keyspace, bool include_only_local_dc) const { +future> storage_proxy::describe_ring(const sstring& keyspace, bool include_only_local_dc) const { return locator::describe_ring(_db.local(), _remote->gossiper(), keyspace, include_only_local_dc); } diff --git a/service/storage_proxy.hh b/service/storage_proxy.hh index b969bf67c1..241d346e47 100644 --- a/service/storage_proxy.hh +++ b/service/storage_proxy.hh @@ -245,7 +245,7 @@ public: // using gossip and by passing the information in each MUTATION_DONE rpc call response. db::view::update_backlog get_backlog_of(locator::host_id) const; - future> describe_ring(const sstring& keyspace, bool include_only_local_dc = false) const; + future> describe_ring(const sstring& keyspace, bool include_only_local_dc = false) const; private: distributed& _db; diff --git a/service/storage_service.cc b/service/storage_service.cc index 80332518e0..fdc2f5119c 100644 --- a/service/storage_service.cc +++ b/service/storage_service.cc @@ -5385,7 +5385,7 @@ future<> storage_service::move(token new_token) { }); } -future> +future> storage_service::describe_ring(const sstring& keyspace, bool include_only_local_dc) const { if (_db.local().find_keyspace(keyspace).uses_tablets()) { throw std::runtime_error(fmt::format("The keyspace {} has tablet table. Query describe_ring with the table parameter!", keyspace)); @@ -5393,7 +5393,7 @@ storage_service::describe_ring(const sstring& keyspace, bool include_only_local_ co_return co_await locator::describe_ring(_db.local(), _gossiper, keyspace, include_only_local_dc); } -future> +future> storage_service::describe_ring_for_table(const sstring& keyspace_name, const sstring& table_name) const { slogger.debug("describe_ring for table {}.{}", keyspace_name, table_name); auto& t = _db.local().find_column_family(keyspace_name, table_name); @@ -5405,7 +5405,7 @@ storage_service::describe_ring_for_table(const sstring& keyspace_name, const sst auto erm = t.get_effective_replication_map(); auto& tmap = erm->get_token_metadata_ptr()->tablets().get_tablet_map(tid); const auto& topology = erm->get_topology(); - std::vector ranges; + utils::chunked_vector ranges; co_await tmap.for_each_tablet([&] (locator::tablet_id id, const locator::tablet_info& info) -> future<> { auto range = tmap.get_token_range(id); auto& replicas = info.replicas; diff --git a/service/storage_service.hh b/service/storage_service.hh index 112c401d70..d43b0e86a3 100644 --- a/service/storage_service.hh +++ b/service/storage_service.hh @@ -467,9 +467,9 @@ public: */ //std::vector describeRingJMX(const sstring& keyspace) const { - future> describe_ring(const sstring& keyspace, bool include_only_local_dc = false) const; + future> describe_ring(const sstring& keyspace, bool include_only_local_dc = false) const; - future> describe_ring_for_table(const sstring& keyspace_name, const sstring& table_name) const; + future> describe_ring_for_table(const sstring& keyspace_name, const sstring& table_name) const; /** * Retrieve a map of tokens to endpoints, including the bootstrapping ones.