Fixed segfault that may happen when data stream is opened during compaction #6619

mmaslankaprv · 2022-10-04T15:24:37Z

Cover letter

Fixed two issues that happened when self compacting segment with open output file streams.

Modifying list when iterating over

When _stream list was iterated and modified in the same loop this
invalidated the reference and caused segmentation fault. Using intrusive
list move constructor instead and then updating the parent pointer in
handler.

Missing custom assignment operator

Added custom move assignment operator to segment_reader as when
assigned all the handlers must be updated with the parent pointer.

Backtrace

void seastar::backtrace<seastar::backtrace_buffer::append_backtrace()::{lambda(seastar::frame)#1}>(seastar::backtrace_buffer::append_backtrace()::{lambda(seastar::frame)#1}&&) at /v/build/v_deps_build/seastar-prefix/src/seastar/include/seastar/util/backtrace.hh:59
 (inlined by) seastar::backtrace_buffer::append_backtrace() at /v/build/v_deps_build/seastar-prefix/src/seastar/src/core/reactor.cc:758
 (inlined by) seastar::print_with_backtrace(seastar::backtrace_buffer&, bool) at /v/build/v_deps_build/seastar-prefix/src/seastar/src/core/reactor.cc:788
seastar::print_with_backtrace(char const*, bool) at /v/build/v_deps_build/seastar-prefix/src/seastar/src/core/reactor.cc:800
 (inlined by) seastar::sigsegv_action() at /v/build/v_deps_build/seastar-prefix/src/seastar/src/core/reactor.cc:3685
 (inlined by) operator() at /v/build/v_deps_build/seastar-prefix/src/seastar/src/core/reactor.cc:3671
 (inlined by) __invoke at /v/build/v_deps_build/seastar-prefix/src/seastar/src/core/reactor.cc:3667
?? ??:0
boost::intrusive::list_node_traits<void*>::set_previous(boost::intrusive::list_node<void*>*, boost::intrusive::list_node<void*>*) at /vectorized/include/boost/intrusive/detail/list_node.hpp:57
 (inlined by) boost::intrusive::circular_list_algorithms<boost::intrusive::list_node_traits<void*> >::link_before(boost::intrusive::list_node<void*>*, boost::intrusive::list_node<void*>*) at /vectorized/include/boost/intrusive/circular_list_algorithms.hpp:181
 (inlined by) boost::intrusive::list_impl<boost::intrusive::mhtraits<storage::segment_reader_handle, boost::intrusive::list_member_hook<boost::intrusive::link_mode<(boost::intrusive::link_mode_type)2> >, &storage::segment_reader_handle::_hook>, unsigned long, false, void>::push_back(storage::segment_reader_handle&) at /vectorized/include/boost/intrusive/list.hpp:272
 (inlined by) segment_reader at /var/lib/buildkite-agent/builds/buildkite-amd64-xfs-builders-i-024fc11f21c7b9fe3-1/redpanda/redpanda/vbuild/release/clang/../../../src/v/storage/segment_reader.cc:62
_ZNSt3__14swapIN7storage14segment_readerEEENS_9enable_ifIXaasr21is_move_constructibleIT_EE5valuesr18is_move_assignableIS4_EE5valueEvE4typeERS4_S7_ at /vectorized/llvm/bin/../include/c++/v1/__utility/swap.h:35
storage::internal::do_swap_data_file_handles(std::__1::__fs::filesystem::path, seastar::lw_shared_ptr<storage::segment>, storage::compaction_config, storage::probe&) at /var/lib/buildkite-agent/builds/buildkite-amd64-xfs-builders-i-024fc11f21c7b9fe3-1/redpanda/redpanda/vbuild/release/clang/../../../src/v/storage/segment_utils.cc:437
std::__1::coroutine_handle<seastar::internal::coroutine_traits_base<void>::promise_type>::resume() const at /vectorized/llvm/bin/../include/c++/v1/__coroutine/coroutine_handle.h:168
 (inlined by) seastar::internal::coroutine_traits_base<void>::promise_type::run_and_dispose() at /vectorized/include/seastar/core/coroutine.hh:120
seastar::reactor::run_tasks(seastar::reactor::task_queue&) at /v/build/v_deps_build/seastar-prefix/src/seastar/src/core/reactor.cc:2356
 (inlined by) seastar::reactor::run_some_tasks() at /v/build/v_deps_build/seastar-prefix/src/seastar/src/core/reactor.cc:2769
seastar::reactor::do_run() at /v/build/v_deps_build/seastar-prefix/src/seastar/src/core/reactor.cc:2938
operator() at /v/build/v_deps_build/seastar-prefix/src/seastar/src/core/reactor.cc:4163
 (inlined by) decltype ((static_cast<seastar::smp::configure(seastar::smp_options const&, seastar::reactor_options const&)::$_94&>({parm#1}))()) std::__1::__invoke<seastar::smp::configure(seastar::smp_options const&, seastar::reactor_options const&)::$_94&>(seastar::smp::configure(seastar::smp_options const&, seastar::reactor_options const&)::$_94&) at /vectorized/llvm/bin/../include/c++/v1/type_traits:3640
 (inlined by) void std::__1::__invoke_void_return_wrapper<void, true>::__call<seastar::smp::configure(seastar::smp_options const&, seastar::reactor_options const&)::$_94&>(seastar::smp::configure(seastar::smp_options const&, seastar::reactor_options const&)::$_94&) at /vectorized/llvm/bin/../include/c++/v1/__functional/invoke.h:61
 (inlined by) std::__1::__function::__alloc_func<seastar::smp::configure(seastar::smp_options const&, seastar::reactor_options const&)::$_94, std::__1::allocator<seastar::smp::configure(seastar::smp_options const&, seastar::reactor_options const&)::$_94>, void ()>::operator()() at /vectorized/llvm/bin/../include/c++/v1/__functional/function.h:180
 (inlined by) std::__1::__function::__func<seastar::smp::configure(seastar::smp_options const&, seastar::reactor_options const&)::$_94, std::__1::allocator<seastar::smp::configure(seastar::smp_options const&, seastar::reactor_options const&)::$_94>, void ()>::operator()() at /vectorized/llvm/bin/../include/c++/v1/__functional/function.h:354
std::__1::__function::__value_func<void ()>::operator()() const at /vectorized/llvm/bin/../include/c++/v1/__functional/function.h:507
 (inlined by) std::__1::function<void ()>::operator()() const at /vectorized/llvm/bin/../include/c++/v1/__functional/function.h:1184
 (inlined by) seastar::posix_thread::start_routine(void*) at /v/build/v_deps_build/seastar-prefix/src/seastar/src/core/posix.cc:60
/opt/redpanda/lib/libpthread.so.0: ELF 64-bit LSB shared object, x86-64, version 1 (SYSV), dynamically linked, interpreter /lib64/ld-linux-x86-64.so.2, BuildID[sha1]=7b0cdaf878ab4f99078439d864af70a5fd7b5a2c, for GNU/Linux 3.2.0, stripped

Fixes #ISSUE-NUMBER, Fixes #ISSUE-NUMBER, ...

Backport Required

UX changes

Describe in plain language how this PR affects an end-user. What topic flags, configuration flags, command line flags, deprecation policies etc are added/changed.

Release notes

none

When `_stream` list was iterated and modified in the same loop this invalidated the reference and caused segmentation fault. Using intrusive list move constructor instead and then updating the parent pointer in handler. Signed-off-by: Michal Maslanka <michal@redpanda.com>

Added custom move assignment operator to `segment_reader` as when assigned all the handlers must be updated with the parent pointer. Signed-off-by: Michal Maslanka <michal@redpanda.com>

Signed-off-by: Michal Maslanka <michal@redpanda.com>

jcsp · 2022-10-04T17:36:32Z

Great catch. This feels like a signal that nothing in our integration tests is pushing compaction hard enough, as it wasn't reproducing there.

dotnwat

move semantics bites again!

mmedenjak · 2022-10-04T19:13:34Z

/backport v22.2.x

vbotbuildovich · 2022-10-04T19:14:46Z

Failed to run cherry-pick command. I executed the below command:

git cherry-pick -x 3f9cc0c9c1d37426a6e4d8526e60974550f6877f a335b0b76ad5568a83fcde109ed8a425391fb0c5 370edc368abf706e2e9bf69e14ff9c7ec26a0d23 57422a21ae5bdbbf80041e387188354dd54d2d38

Workflow run logs.

[v22.2.x] Backport of #6619

mmaslankaprv requested review from jcsp and dotnwat October 4, 2022 15:24

github-actions bot added the area/redpanda label Oct 4, 2022

mmaslankaprv force-pushed the fix-seg-fault-when-compacting branch from b54a494 to 5696bfc Compare October 4, 2022 15:28

mmaslankaprv added 3 commits October 4, 2022 18:22

s/segment_reader: added custom move assignment operator

a335b0b

Added custom move assignment operator to `segment_reader` as when assigned all the handlers must be updated with the parent pointer. Signed-off-by: Michal Maslanka <michal@redpanda.com>

s/tests: added test checking compaction while keeping a stream opened

370edc3

Signed-off-by: Michal Maslanka <michal@redpanda.com>

mmaslankaprv force-pushed the fix-seg-fault-when-compacting branch from 5696bfc to f30c355 Compare October 4, 2022 16:23

tests: set default segment size jitter in tests

57422a2

Signed-off-by: Michal Maslanka <michal@redpanda.com>

mmaslankaprv force-pushed the fix-seg-fault-when-compacting branch from f30c355 to 57422a2 Compare October 4, 2022 16:28

jcsp mentioned this pull request Oct 4, 2022

Assertion thrown during compaction #5132

Closed

jcsp approved these changes Oct 4, 2022

View reviewed changes

dotnwat approved these changes Oct 4, 2022

View reviewed changes

mmedenjak merged commit a8c60ea into redpanda-data:dev Oct 4, 2022

mmaslankaprv mentioned this pull request Oct 5, 2022

[v22.2.x] Backport of #6619 #6632

Merged

mmedenjak added kind/bug Something isn't working area/storage and removed area/redpanda labels Oct 5, 2022

mmedenjak pushed a commit that referenced this pull request Oct 5, 2022

Merge pull request #6632 from mmaslankaprv/v22.2.x

3420754

[v22.2.x] Backport of #6619

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fixed segfault that may happen when data stream is opened during compaction #6619

Fixed segfault that may happen when data stream is opened during compaction #6619

mmaslankaprv commented Oct 4, 2022 •

edited

Loading

jcsp commented Oct 4, 2022

dotnwat left a comment

mmedenjak commented Oct 4, 2022

vbotbuildovich commented Oct 4, 2022

Fixed segfault that may happen when data stream is opened during compaction #6619

Fixed segfault that may happen when data stream is opened during compaction #6619

Conversation

mmaslankaprv commented Oct 4, 2022 • edited Loading

Cover letter

Modifying list when iterating over

Missing custom assignment operator

Backtrace

Backport Required

UX changes

Release notes

jcsp commented Oct 4, 2022

dotnwat left a comment

Choose a reason for hiding this comment

mmedenjak commented Oct 4, 2022

vbotbuildovich commented Oct 4, 2022

mmaslankaprv commented Oct 4, 2022 •

edited

Loading