From ce45ffdffb23290423f70bdc935036cd30a0eadf Mon Sep 17 00:00:00 2001 From: Calle Wilund Date: Tue, 6 Jul 2021 11:21:04 +0000 Subject: [PATCH] commitlog: Use defensive copies of segment list in iterations Fixes #8952 In 5ebf5835b0c9c3fb929e2ad4657af36515bcc9ad we added a segment prune after flushing, to deal with deadlocks in shutdown. This means that calls that issue sync/flush-like ops "for-all", need to operate on a defensive copy of the list. Closes #8980 --- db/commitlog/commitlog.cc | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/db/commitlog/commitlog.cc b/db/commitlog/commitlog.cc index b7339c4659..d0b01212d4 100644 --- a/db/commitlog/commitlog.cc +++ b/db/commitlog/commitlog.cc @@ -1730,7 +1730,10 @@ future<> db::commitlog::segment_manager::clear_reserve_segments() { future<> db::commitlog::segment_manager::sync_all_segments() { clogger.debug("Issuing sync for all segments"); - return parallel_for_each(_segments, [] (sseg_ptr s) { + // #8952 - calls that do sync/cycle can end up altering + // _segments (end_flush()->discard_unused()) + auto def_copy = _segments; + return parallel_for_each(def_copy, [] (sseg_ptr s) { return s->sync().then([](sseg_ptr s) { clogger.debug("Synced segment {}", *s); }); @@ -1739,7 +1742,10 @@ future<> db::commitlog::segment_manager::sync_all_segments() { future<> db::commitlog::segment_manager::shutdown_all_segments() { clogger.debug("Issuing shutdown for all segments"); - return parallel_for_each(_segments, [] (sseg_ptr s) { + // #8952 - calls that do sync/cycle can end up altering + // _segments (end_flush()->discard_unused()) + auto def_copy = _segments; + return parallel_for_each(def_copy, [] (sseg_ptr s) { return s->shutdown().then([](sseg_ptr s) { clogger.debug("Shutdown segment {}", *s); }); @@ -1946,7 +1952,10 @@ future<> db::commitlog::segment_manager::clear() { */ void db::commitlog::segment_manager::sync() { auto f = std::exchange(_background_sync, make_ready_future<>()); - _background_sync = parallel_for_each(_segments, [](sseg_ptr s) { + // #8952 - calls that do sync/cycle can end up altering + // _segments (end_flush()->discard_unused()) + auto def_copy = _segments; + _background_sync = parallel_for_each(def_copy, [](sseg_ptr s) { return s->sync().discard_result(); }).then([f = std::move(f)]() mutable { return std::move(f);