core: use 64 bit head+tail pointers in fair_queue #10

jcsp · 2021-08-31T19:11:28Z

This is the near-term fix for hanging IOs under heavy load. The switch from uint32_t to uint64_t should be cheap on modern x86_64 processors, but may be more expensive on other archs (I'm not sure what the deal is with the various ARM archs and 8 byte atomics).

When uint32_t offsets were used here, the frequent
integer wrapping caused queues to get stuck. The _pending
tail values would compare as greater than the head values
forever if there weren't always new requests coming through
to wrap the head/tail pointers again.

Signed-off-by: John Spray jcs@vectorized.io

When uint32_t offsets were used here, the frequent integer wrapping caused queues to get stuck. The _pending tail values would compare as greater than the head values forever if there weren't always new requests coming through to wrap the head/tail pointers again. Signed-off-by: John Spray <jcs@vectorized.io>

CLAassistant · 2021-08-31T19:11:33Z

All committers have signed the CLA.

dotnwat · 2021-08-31T22:45:52Z

src/core/fair_queue.cc

-    return fair_queue_ticket(std::max<int32_t>(_weight - other._weight, 0),
-            std::max<int32_t>(_size - other._size, 0));
+
+    uint64_t weight_d = std::clamp<int64_t>(_weight - other._weight, 0, std::numeric_limits<int32_t>::max());


is _weight - other._weight unsigned? if so, doesn't that mean it will never compare less than 0 and this entire expression is equivalent to min(int32::max, weight-other.weight)?

edit: ohhh, does it cast to that signed int64_t template parameter before any of the calculations? wild interface

…o_with Fixes failures in debug mode: ``` $ build/debug/tests/unit/closeable_test -l all -t deferred_close_test WARNING: debug mode. Not for benchmarking or production random-seed=3064133628 Running 1 test case... Entering test module "../../tests/unit/closeable_test.cc" ../../tests/unit/closeable_test.cc(0): Entering test case "deferred_close_test" ../../src/testing/seastar_test.cc(43): info: check true has passed ==9449==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases! terminate called after throwing an instance of 'seastar::broken_promise' what(): broken promise ==9449==WARNING: ASan is ignoring requested __asan_handle_no_return: stack top: 0x7fbf1f49f000; bottom 0x7fbf40971000; size: 0xffffffffdeb2e000 (-558702592) False positive error reports may follow For details see google/sanitizers#189 ================================================================= ==9449==AddressSanitizer CHECK failed: ../../../../libsanitizer/asan/asan_thread.cpp:356 "((ptr[0] == kCurrentStackFrameMagic)) != (0)" (0x0, 0x0) #0 0x7fbf45f39d0b (/lib64/libasan.so.6+0xb3d0b) redpanda-data#1 0x7fbf45f57d4e (/lib64/libasan.so.6+0xd1d4e) redpanda-data#2 0x7fbf45f3e724 (/lib64/libasan.so.6+0xb8724) redpanda-data#3 0x7fbf45eb3e5b (/lib64/libasan.so.6+0x2de5b) redpanda-data#4 0x7fbf45eb51e8 (/lib64/libasan.so.6+0x2f1e8) redpanda-data#5 0x7fbf45eb7694 (/lib64/libasan.so.6+0x31694) redpanda-data#6 0x7fbf45f39398 (/lib64/libasan.so.6+0xb3398) redpanda-data#7 0x7fbf45f3a00b in __asan_report_load8 (/lib64/libasan.so.6+0xb400b) redpanda-data#8 0xfe6d52 in bool __gnu_cxx::operator!=<dl_phdr_info*, std::vector<dl_phdr_info, std::allocator<dl_phdr_info> > >(__gnu_cxx::__normal_iterator<dl_phdr_info*, std::vector<dl_phdr_info, std::allocator<dl_phdr_info> > > const&, __gnu_cxx::__normal_iterator<dl_phdr_info*, std::vector<dl_phdr_info, std::allocator<dl_phdr_info> > > const&) /usr/include/c++/10/bits/stl_iterator.h:1116 redpanda-data#9 0xfe615c in dl_iterate_phdr ../../src/core/exception_hacks.cc:121 redpanda-data#10 0x7fbf44bd1810 in _Unwind_Find_FDE (/lib64/libgcc_s.so.1+0x13810) redpanda-data#11 0x7fbf44bcd897 (/lib64/libgcc_s.so.1+0xf897) redpanda-data#12 0x7fbf44bcea5f (/lib64/libgcc_s.so.1+0x10a5f) redpanda-data#13 0x7fbf44bcefd8 in _Unwind_RaiseException (/lib64/libgcc_s.so.1+0x10fd8) redpanda-data#14 0xfe6281 in _Unwind_RaiseException ../../src/core/exception_hacks.cc:148 redpanda-data#15 0x7fbf457364bb in __cxa_throw (/lib64/libstdc++.so.6+0xaa4bb) redpanda-data#16 0x7fbf45e10a21 (/lib64/libboost_unit_test_framework.so.1.73.0+0x1aa21) redpanda-data#17 0x7fbf45e20fe0 in boost::execution_monitor::execute(boost::function<int ()> const&) (/lib64/libboost_unit_test_framework.so.1.73.0+0x2afe0) redpanda-data#18 0x7fbf45e21094 in boost::execution_monitor::vexecute(boost::function<void ()> const&) (/lib64/libboost_unit_test_framework.so.1.73.0+0x2b094) redpanda-data#19 0x7fbf45e43921 in boost::unit_test::unit_test_monitor_t::execute_and_translate(boost::function<void ()> const&, unsigned long) (/lib64/libboost_unit_test_framework.so.1.73.0+0x4d921) redpanda-data#20 0x7fbf45e5eae1 (/lib64/libboost_unit_test_framework.so.1.73.0+0x68ae1) redpanda-data#21 0x7fbf45e5ed31 (/lib64/libboost_unit_test_framework.so.1.73.0+0x68d31) redpanda-data#22 0x7fbf45e2e547 in boost::unit_test::framework::run(unsigned long, bool) (/lib64/libboost_unit_test_framework.so.1.73.0+0x38547) redpanda-data#23 0x7fbf45e43618 in boost::unit_test::unit_test_main(bool (*)(), int, char**) (/lib64/libboost_unit_test_framework.so.1.73.0+0x4d618) redpanda-data#24 0x44798d in seastar::testing::entry_point(int, char**) ../../src/testing/entry_point.cc:77 redpanda-data#25 0x4134b5 in main ../../include/seastar/testing/seastar_test.hh:65 redpanda-data#26 0x7fbf44a1b1e1 in __libc_start_main (/lib64/libc.so.6+0x281e1) redpanda-data#27 0x4133dd in _start (/home/bhalevy/dev/seastar/build/debug/tests/unit/closeable_test+0x4133dd) ``` Signed-off-by: Benny Halevy <bhalevy@scylladb.com> Message-Id: <20210406100911.12278-1-bhalevy@scylladb.com>

in main(), we creates an instance of `http_server_control` using new, but we never destroy it. this is identified by ASan ``` ==2190125==ERROR: LeakSanitizer: detected memory leaks Direct leak of 8 byte(s) in 1 object(s) allocated from: #0 0x55e21cf487bd in operator new(unsigned long) /home/kefu/dev/llvm-project/compiler-rt/lib/asan/asan_new_delete.cpp:86:3 redpanda-data#1 0x55e21cf6cf31 in main::$_0::operator()() const::'lambda'()::operator()() const /home/kefu/dev/seastar/apps/httpd/main.cc:121:27 redpanda-data#2 0x55e21cf6b4cc in int std::__invoke_impl<int, main::$_0::operator()() const::'lambda'()>(std::__invoke_other, main::$_0::operator()() const::'lambda'()&&) /usr/lib/gcc/x86_64-redhat-linux/14/../../../../incl ude/c++/14/bits/invoke.h:61:14 redpanda-data#3 0x55e21cf6b46c in std::__invoke_result<main::$_0::operator()() const::'lambda'()>::type std::__invoke<main::$_0::operator()() const::'lambda'()>(main::$_0::operator()() const::'lambda'()&&) /usr/lib/gcc/x86_ 64-redhat-linux/14/../../../../include/c++/14/bits/invoke.h:96:14 redpanda-data#4 0x55e21cf6b410 in decltype(auto) std::__apply_impl<main::$_0::operator()() const::'lambda'(), std::tuple<>>(main::$_0::operator()() const::'lambda'()&&, std::tuple<>&&, std::integer_sequence<unsigned long, . ..>) /usr/lib/gcc/x86_64-redhat-linux/14/../../../../include/c++/14/tuple:2921:14 redpanda-data#5 0x55e21cf6b3b2 in decltype(auto) std::apply<main::$_0::operator()() const::'lambda'(), std::tuple<>>(main::$_0::operator()() const::'lambda'()&&, std::tuple<>&&) /usr/lib/gcc/x86_64-redhat-linux/14/../../../ ../include/c++/14/tuple:2936:14 redpanda-data#6 0x55e21cf6b283 in seastar::future<int> seastar::futurize<int>::apply<main::$_0::operator()() const::'lambda'()>(main::$_0::operator()() const::'lambda'()&&, std::tuple<>&&) /home/kefu/dev/seastar/include/sea star/core/future.hh:2005:28 redpanda-data#7 0x55e21cf6b043 in seastar::futurize<std::invoke_result<main::$_0::operator()() const::'lambda'()>::type>::type seastar::async<main::$_0::operator()() const::'lambda'()>(seastar::thread_attributes, main::$_0: :operator()() const::'lambda'()&&)::'lambda'()::operator()() const /home/kefu/dev/seastar/include/seastar/core/thread.hh:260:13 redpanda-data#8 0x55e21cf6ae74 in seastar::noncopyable_function<void ()>::direct_vtable_for<seastar::futurize<std::invoke_result<main::$_0::operator()() const::'lambda'()>::type>::type seastar::async<main::$_0::operator()() const::'lambda'()>(seastar::thread_attributes, main::$_0::operator()() const::'lambda'()&&)::'lambda'()>::call(seastar::noncopyable_function<void ()> const*) /home/kefu/dev/seastar/include/seastar/util/noncopyable _function.hh:129:20 redpanda-data#9 0x7f5d757a0fb3 in seastar::noncopyable_function<void ()>::operator()() const /home/kefu/dev/seastar/include/seastar/util/noncopyable_function.hh:215:16 redpanda-data#10 0x7f5d75ef5611 in seastar::thread_context::main() /home/kefu/dev/seastar/src/core/thread.cc:311:9 redpanda-data#11 0x7f5d75ef50eb in seastar::thread_context::s_main(int, int) /home/kefu/dev/seastar/src/core/thread.cc:287:43 redpanda-data#12 0x7f5d72f8a18f (/lib64/libc.so.6+0x5a18f) (BuildId: b098f1c75a76548bb230d8f551eae07a2aeccf06) ``` so, in this change, let's hold it using a smart pointer, so we can destroy it when it leaves the lexical scope. Signed-off-by: Kefu Chai <kefu.chai@scylladb.com> Closes scylladb#2224

dotnwat approved these changes Aug 31, 2021

View reviewed changes

dotnwat merged commit 48f7ad9 into redpanda-data:master Sep 1, 2021

jcsp deleted the wip-fairqueue-1 branch September 1, 2021 08:44

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

core: use 64 bit head+tail pointers in fair_queue #10

core: use 64 bit head+tail pointers in fair_queue #10

jcsp commented Aug 31, 2021

CLAassistant commented Aug 31, 2021 •

edited

Loading

dotnwat Aug 31, 2021

core: use 64 bit head+tail pointers in fair_queue #10

core: use 64 bit head+tail pointers in fair_queue #10

Conversation

jcsp commented Aug 31, 2021

CLAassistant commented Aug 31, 2021 • edited Loading

dotnwat Aug 31, 2021

Choose a reason for hiding this comment

CLAassistant commented Aug 31, 2021 •

edited

Loading