From 53fee789f0a95ddbed44089b4010e86e93d9e69f Mon Sep 17 00:00:00 2001 From: Asias He Date: Mon, 10 Aug 2020 10:47:45 +0800 Subject: [PATCH] repair: Use merge_to_gently to merge two lists During a performance test, test_latency_read_with_nemesis during manager repair, it experienced a stall of 73 ms: ``` (inlined by) std::back_insert_iterator > >::operator=(repair_row const&) at /usr/include/c++/9/bits/stl_iterator.h:515 (inlined by) std::back_insert_iterator > > std::__copy_move::__copy_m, std::back_insert_iterator > > >(std::_List_iterator, std::_List_iterator, std::back_insert_iterator > >) at /usr/include/c++/9/bits/stl_algobase.h:312 (inlined by) std::back_insert_iterator > > std::__copy_move_a, std::back_insert_iterator > > >(std::_List_iterator, std::_List_iterator, std::back_insert_iterator > >) at /usr/include/c++/9/bits/stl_algobase.h:404 (inlined by) std::back_insert_iterator > > std::__copy_move_a2, std::back_insert_iterator > > >(std::_List_iterator, std::_List_iterator, std::back_insert_iterator > >) at /usr/include/c++/9/bits/stl_algobase.h:440 (inlined by) std::back_insert_iterator > > std::copy, std::back_insert_iterator > > >(std::_List_iterator, std::_List_iterator, std::back_insert_iterator > >) at /usr/include/c++/9/bits/stl_algobase.h:474 (inlined by) std::back_insert_iterator > > std::__merge, std::_List_iterator, std::back_insert_iterator > >, __gnu_cxx::__ops::_Iter_comp_iter >, gms::inet_address, seastar::bool_class, seastar::bool_class, unsigned int)::{lambda(repair_row const&, repair_row const&)#1}> >(std::_List_iterator, std::back_insert_iterator > >, std::_List_iterator, std::_List_iterator, __gnu_cxx::__ops::_Iter_comp_iter >, gms::inet_address, seastar::bool_class, seastar::bool_class, unsigned int)::{lambda(repair_row const&, repair_row const&)#1}>, __gnu_cxx::__ops::_Iter_comp_iter >, gms::inet_address, seastar::bool_class, seastar::bool_class, unsigned int)::{lambda(repair_row const&, repair_row const&)#1}>) at /usr/include/c++/9/bits/stl_algo.h:4923 (inlined by) std::back_insert_iterator > > std::merge, std::_List_iterator, std::back_insert_iterator > >, repair_meta::apply_rows_on_master_in_thread(std::__cxx11::list >, gms::inet_address, seastar::bool_class, seastar::bool_class, unsigned int)::{lambda(repair_row const&, repair_row const&)#1}>(std::_List_iterator, std::back_insert_iterator > >, std::_List_iterator, std::_List_iterator, repair_meta::apply_rows_on_master_in_thread(std::__cxx11::list >, gms::inet_address, seastar::bool_class, seastar::bool_class, unsigned int)::{lambda(repair_row const&, repair_row const&)#1}, repair_meta::apply_rows_on_master_in_thread(std::__cxx11::list >, gms::inet_address, seastar::bool_class, seastar::bool_class, unsigned int)::{lambda(repair_row const&, repair_row const&)#1}) at /usr/include/c++/9/bits/stl_algo.h:5018 (inlined by) repair_meta::apply_rows_on_master_in_thread(std::__cxx11::list >, gms::inet_address, seastar::bool_class, seastar::bool_class, unsigned int) at ./repair/row_level.cc:1242 repair_meta::get_row_diff_source_op(seastar::bool_class, gms::inet_address, unsigned int, seastar::rpc::sink&, seastar::rpc::source&) at ./repair/row_level.cc:1608 repair_meta::get_row_diff_with_rpc_stream(std::unordered_set, std::equal_to, std::allocator >, seastar::bool_class, seastar::bool_class, gms::inet_address, unsigned int) at ./repair/row_level.cc:1674 row_level_repair::get_missing_rows_from_follower_nodes(repair_meta&) at ./repair/row_level.cc:2413 ``` The problem was that when std::merge() ran out of one range, it copied the second range. To fix, use the new merge_to_gently helper. Fixes #6976 --- repair/row_level.cc | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/repair/row_level.cc b/repair/row_level.cc index 985690c339..bab18bd8f9 100644 --- a/repair/row_level.cc +++ b/repair/row_level.cc @@ -48,6 +48,7 @@ #include "gms/gossiper.hh" #include "repair/row_level.hh" #include "mutation_source_metadata.hh" +#include "utils/stall_free.hh" extern logging::logger rlogger; @@ -1261,13 +1262,11 @@ private: stats().rx_row_nr += row_diff.size(); stats().rx_row_nr_peer[from] += row_diff.size(); if (update_buf) { - std::list tmp; - tmp.swap(_working_row_buf); // Both row_diff and _working_row_buf and are ordered, merging // two sored list to make sure the combination of row_diff // and _working_row_buf are ordered. - std::merge(tmp.begin(), tmp.end(), row_diff.begin(), row_diff.end(), std::back_inserter(_working_row_buf), - [this] (const repair_row& x, const repair_row& y) { thread::maybe_yield(); return _cmp(x.boundary(), y.boundary()) < 0; }); + utils::merge_to_gently(_working_row_buf, row_diff, + [this] (const repair_row& x, const repair_row& y) { return _cmp(x.boundary(), y.boundary()) < 0; }); } if (update_hash_set) { _peer_row_hash_sets[node_idx] = boost::copy_range(row_diff |