From 840ca4139326bc3ee9398fbe8a1b124ebe668e54 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Botond=20D=C3=A9nes?= Date: Wed, 28 Apr 2021 16:07:24 +0300 Subject: [PATCH] database: clear inactive reads in stop() If any inactive read is left in the semaphore, it can block `database::stop()` from shutting down, as sstables pinned by these reads will prevent `sstables::sstables_manager::close()` from finishing. This causes a deadlock. It is not clear how inactive reads can be left in the semaphore, as all users are supposed to clean up after themselves. Post 4.4 releases don't have this problem anymore as the inactive read handle was made a RAII object, removing the associated inactive read when destroyed. In 4.4 and earlier release this wasn't so, so errors could be made. Normally this is not a big issue, as these orphaned inactive reads are just evicted when the resources they own are needed, but it does become a serious issue during shutdown. To prevent a deadlock, clear the inactive reads earlier, in `database::stop()` (currently they are cleared in the destructor). This is a simple and foolproof way of ensuring any leftover inactive reads don't cause problems. Fixes: #8561 Tests: unit(dev) Closes #8562 --- database.cc | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/database.cc b/database.cc index 452e3f7f35..9b2d813bc5 100644 --- a/database.cc +++ b/database.cc @@ -760,9 +760,6 @@ void database::set_format_by_config() { } database::~database() { - _read_concurrency_sem.clear_inactive_reads(); - _streaming_concurrency_sem.clear_inactive_reads(); - _system_read_concurrency_sem.clear_inactive_reads(); } void database::update_version(const utils::UUID& version) { @@ -2016,6 +2013,13 @@ future<> database::stop() { assert(!_large_data_handler->running()); + // Inactive reads might hold on to sstables, blocking the + // `sstables_manager::close()` calls below. No one will come back for these + // reads at this point so clear them before proceeding with the shutdown. + _read_concurrency_sem.clear_inactive_reads(); + _streaming_concurrency_sem.clear_inactive_reads(); + _system_read_concurrency_sem.clear_inactive_reads(); + // try to ensure that CL has done disk flushing future<> maybe_shutdown_commitlog = _commitlog != nullptr ? _commitlog->shutdown() : make_ready_future<>(); return maybe_shutdown_commitlog.then([this] {