Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SIGSEGV in db::hints::space_watchdog #4685

Closed
tgrabiec opened this issue Jul 9, 2019 · 9 comments
Closed

SIGSEGV in db::hints::space_watchdog #4685

tgrabiec opened this issue Jul 9, 2019 · 9 comments

Comments

@tgrabiec
Copy link
Contributor

@tgrabiec tgrabiec commented Jul 9, 2019

Installation details
Scylla version (or git commit hash): 3.1.0.rc2-0.20190630.acff367ea

Observed in longevity run from #4647

$ eu-addr2line --demangle --functions --symbols --pretty-print -e /usr/lib/debug/opt/scylladb/libexec/scylla.bin-3.1.0.rc2-0.20190630.acff367ea.el7.x86_64.debug
0x0000000004164e42
0x00000000040520c5
0x00000000040523c5
0x0000000004052413
0x00007ff358c9302f
0x00000000024a32b9
0x000000000404f5b1
0x000000000404f7ae
0x000000000413147d
0x000000000415baeb
0x000000000401d8ad

seastar::circular_buffer<seastar::shared_mutex::waiter, std::allocator<seastar::shared_mutex::waiter> >::front() seastar::continuation<seastar::future<> seastar::future<>::then_wrapped_impl<seastar::future<>::finally_body<seastar::futurize<std::result_of<db::hints::space_watchdog::on_timer()::{lambda(std::filesystem::__cxx11::path, seastar::directory_entry)#1}::operator()(std::filesystem::__cxx11::path, seastar::directory_entry) const::{lambda()#1} ()>::type>::type seastar::with_lock<db::hints::space_watchdog::on_timer()::{lambda(std::filesystem::__cxx11::path, seastar::directory_entry)#1}::operator()(std::filesystem::__cxx11::path, seastar::directory_entry) const::{lambda()#1}>(seastar::shared_mutex&, seastar::futurize&&)::{lambda()#2}, false>, seastar::future<> >(seastar::shared_mutex)::{lambda()#1}::operator()() const::{lambda(seastar::shared_mutex)#1}>::run_and_dispose()+0x129 at /jenkins/workspace/scylla-3.1/relocatable-pkg/scylla/seastar/include/seastar/core/circular_buffer.hh:401:43
seastar::reactor::run_tasks(seastar::reactor::task_queue&) seastar::reactor::run_tasks(seastar::reactor::task_queue&)+0x41 at ../../src/core/reactor.cc:3670:29
seastar::reactor::run_some_tasks() seastar::reactor::run_some_tasks() [clone .part.2859]+0x1ae at ../../src/core/reactor.cc:4095:18
seastar::reactor::run_some_tasks() seastar::reactor::run()+0x112d at ../../src/core/reactor.cc:4078:5
seastar::smp::configure(boost::program_options::variables_map, seastar::reactor_config)::{lambda()#3}::operator()() const+0xb5b at ../../src/core/reactor.cc:5416:25
std::function<void ()>::operator()() const seastar::posix_thread::start_routine(void*)+0xd at /usr/include/c++/8/bits/std_function.h:687:14
@tgrabiec tgrabiec added the bug label Jul 9, 2019
@tgrabiec

This comment has been minimized.

Copy link
Contributor Author

@tgrabiec tgrabiec commented Jul 10, 2019

The coredump had been uploaded to https://console.cloud.google.com/storage/browser/upload.scylladb.com/core.scylla.996.55289e02c57c4572841d001538c38d31.6684.1562354735000000.gz/?project=upload-179716&authuser=0

$ less longevity-tls-50gb-4d-3-1-repl-db-node-b453a89d-1/coredump.log 
           PID: 6684 (scylla)
           UID: 996 (scylla)
           GID: 1001 (scylla)
        Signal: 11 (SEGV)
     Timestamp: Fri 2019-07-05 19:25:35 UTC (1min 49s ago)
  Command Line: /usr/bin/scylla /opt/scylladb/bin/../libexec/scylla.bin --blocked-reactor-notify-ms 500 --abort-on-lsa-bad-alloc 1 --abort-on-seastar-bad-alloc --log-to-syslog 1 --log-to-stdout 0 --default-log-level info --network-stack posix --io-properties-file=/etc/scylla.d/io_properties.yaml --cpuset 1-7,9-15
    Executable: /opt/scylladb/libreloc/ld.so
 Control Group: /
       Boot ID: 55289e02c57c4572841d001538c38d31
    Machine ID: df877a200226bc47d06f26dae0736ec9
      Hostname: ip-10-0-184-92.eu-west-1.compute.internal
      Coredump: /var/lib/systemd/coredump/core.scylla.996.55289e02c57c4572841d001538c38d31.6684.1562354735000000
       Message: Process 6684 (scylla) of user 996 dumped core.
                
                Stack trace of thread 6695:
                #0  0x00000000024a32ba _ZN7seastar12shared_mutex4wakeEv (/opt/scylladb/libexec/scylla.bin)
@slivne slivne added this to the 3.1 milestone Jul 10, 2019
@slivne

This comment has been minimized.

Copy link
Contributor

@slivne slivne commented Jul 16, 2019

@gleb-cloudius

This comment has been minimized.

Copy link
Contributor

@gleb-cloudius gleb-cloudius commented Jul 17, 2019

Is there log for the run?

@gleb-cloudius

This comment has been minimized.

Copy link
Contributor

@gleb-cloudius gleb-cloudius commented Jul 17, 2019

Also which OS that was running on?

@slivne

This comment has been minimized.

Copy link
Contributor

@slivne slivne commented Jul 17, 2019

@gleb-cloudius

This comment has been minimized.

Copy link
Contributor

@gleb-cloudius gleb-cloudius commented Jul 17, 2019

@gleb-cloudius

This comment has been minimized.

Copy link
Contributor

@gleb-cloudius gleb-cloudius commented Jul 17, 2019

I do not think this is a regression. @vladzcloudius will look into it.

@slivne

This comment has been minimized.

Copy link
Contributor

@slivne slivne commented Jul 17, 2019

Not a regression --> moved to 3.2 with backport request - thanks

@vladzcloudius

This comment has been minimized.

Copy link
Contributor

@vladzcloudius vladzcloudius commented Jul 17, 2019

@slivne I confirm - this is not a regression per se. The bug is there since I introduced the draining logic in

commit 48c96d09d640482f47301f30c633c1c45313be83
Author: Vlad Zolotarov <vladz@scylladb.com>
Date:   Thu Apr 12 20:17:52 2018 -0400

    db::hints::manager: drain hints when the node is decommissioned/removed
    

It looks like I forgot to cover this race.
I'll send a patch.

vladzcloudius pushed a commit to vladzcloudius/scylla that referenced this issue Aug 12, 2019
…emaphore

When elements of manager::_ep_managers map or endpoints' directories are
deleted we want to serialize such events with other asynchronous parts of the code
that may be using these elements or rely on existence of these directories.

There are a few races that may occur if these events are not serialized:
 - scylladb#4685: space watchdog takes a per end_point_hints_manager shared_lock
   and if the corresponding end_point_hints_manager object is being destroyed
   while space watchdog is waiting for this lock there's going to be a
   use-after-free event.
 - Space watchdog scans for per-destination directories and if it's deleted
   while it does that it may lead to an error.
 - If drain_for() is running together with itself: one instance for the local
   node and one for some other node, erasing of elements from the _ep_managers
   map may again lead to a use-after-free event.

In this patch we introduce a semaphore that is going to serialize the context
where such deletions can take place and the context that can be accessing
these elements during the deletion.

Fixes scylladb#4685

Signed-off-by: Vlad Zolotarov <vladz@scylladb.com>
avikivity added a commit that referenced this issue Oct 3, 2019
…Vlad

"
Fix races that may lead to use-after-free events and file system level exceptions
during shutdown and drain.

The root cause of use-after-free events in question is that space_watchdog blocks on
end_point_hints_manager::file_update_mutex() and we need to make sure this mutex is alive as long as
it's accessed even if the corresponding end_point_hints_manager instance
is destroyed in the context of manager::drain_for().

File system exceptions may occur when space_watchdog attempts to scan a
directory while it's being deleted from the drain_for() context.
In case of such an exception new hints generation is going to be blocked
- including for materialized views, till the next space_watchdog round (in 1s).

Issues that are fixed are #4685 and #4836.

Tested as follows:
 1) Patched the code in order to trigger the race with (a lot) higher
    probability and running slightly modified hinted handoff replace
    dtest with a debug binary for 100 times. Side effect of this
    testing was discovering of #4836.
 2) Using the same patch as above tested that there are no crashes and
    nodes survive stop/start sequences (they were not without this series)
    in the context of all hinted handoff dtests. Ran the whole set of
    tests with dev binary for 10 times.
"

* 'hinted_handoff_race_between_drain_for_and_space_watchdog_no_global_lock-v2' of https://github.com/vladzcloudius/scylla:
  hinted handoff: fix a race on a directory removal between space_watchdog and drain_for()
  hinted handoff: make taking file_update_mutex safe
  db::hints::manager::drain_for(): fix alignment
  db::hints::manager: serialize calls to drain_for()
  db::hints: cosmetics: identation and missing method qualifier
avikivity added a commit that referenced this issue Oct 5, 2019
…Vlad

"
Fix races that may lead to use-after-free events and file system level exceptions
during shutdown and drain.

The root cause of use-after-free events in question is that space_watchdog blocks on
end_point_hints_manager::file_update_mutex() and we need to make sure this mutex is alive as long as
it's accessed even if the corresponding end_point_hints_manager instance
is destroyed in the context of manager::drain_for().

File system exceptions may occur when space_watchdog attempts to scan a
directory while it's being deleted from the drain_for() context.
In case of such an exception new hints generation is going to be blocked
- including for materialized views, till the next space_watchdog round (in 1s).

Issues that are fixed are #4685 and #4836.

Tested as follows:
 1) Patched the code in order to trigger the race with (a lot) higher
    probability and running slightly modified hinted handoff replace
    dtest with a debug binary for 100 times. Side effect of this
    testing was discovering of #4836.
 2) Using the same patch as above tested that there are no crashes and
    nodes survive stop/start sequences (they were not without this series)
    in the context of all hinted handoff dtests. Ran the whole set of
    tests with dev binary for 10 times.
"

Fixes #4685
Fixes #4836

* 'hinted_handoff_race_between_drain_for_and_space_watchdog_no_global_lock-v2' of https://github.com/vladzcloudius/scylla:
  hinted handoff: fix a race on a directory removal between space_watchdog and drain_for()
  hinted handoff: make taking file_update_mutex safe
  db::hints::manager::drain_for(): fix alignment
  db::hints::manager: serialize calls to drain_for()
  db::hints: cosmetics: identation and missing method qualifier

(cherry picked from commit 3cb081e)
avikivity added a commit that referenced this issue Oct 5, 2019
…Vlad

"
Fix races that may lead to use-after-free events and file system level exceptions
during shutdown and drain.

The root cause of use-after-free events in question is that space_watchdog blocks on
end_point_hints_manager::file_update_mutex() and we need to make sure this mutex is alive as long as
it's accessed even if the corresponding end_point_hints_manager instance
is destroyed in the context of manager::drain_for().

File system exceptions may occur when space_watchdog attempts to scan a
directory while it's being deleted from the drain_for() context.
In case of such an exception new hints generation is going to be blocked
- including for materialized views, till the next space_watchdog round (in 1s).

Issues that are fixed are #4685 and #4836.

Tested as follows:
 1) Patched the code in order to trigger the race with (a lot) higher
    probability and running slightly modified hinted handoff replace
    dtest with a debug binary for 100 times. Side effect of this
    testing was discovering of #4836.
 2) Using the same patch as above tested that there are no crashes and
    nodes survive stop/start sequences (they were not without this series)
    in the context of all hinted handoff dtests. Ran the whole set of
    tests with dev binary for 10 times.
"

Fixes #4685
Fixes #4836.

* 'hinted_handoff_race_between_drain_for_and_space_watchdog_no_global_lock-v2' of https://github.com/vladzcloudius/scylla:
  hinted handoff: fix a race on a directory removal between space_watchdog and drain_for()
  hinted handoff: make taking file_update_mutex safe
  db::hints::manager::drain_for(): fix alignment
  db::hints::manager: serialize calls to drain_for()
  db::hints: cosmetics: identation and missing method qualifier

(cherry picked from commit 3cb081e)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.