Skip to content

Commit

Permalink
reader_concurrency_semaphore: Fix stop() in face of evictable reads b…
Browse files Browse the repository at this point in the history
…ecoming inactive

Scylla can crash due to a complicated interaction of service level drop,
evictable readers, inactive read registration path.

1) service level drop invoke stop of reader concurrency semaphore, which will
wait for in flight requests

2) turns out it stops first the gate used for closing readers that will
become inactive.

3) proceeds to wait for in-flight reads by closing the reader permit gate.

4) one of evictable reads take the inactive read registration path, and
finds the gate for closing readers closed.

5) flat mutation reader is destroyed, but finds the underlying reader was
not closed gracefully and triggers the abort.

By closing permit gate first, evictable readers becoming inactive will
be able to properly close underlying reader, therefore avoiding the
crash.

Fixes #15534.

Signed-off-by: Raphael S. Carvalho <raphaelsc@scylladb.com>

Closes #15535
  • Loading branch information
raphaelsc authored and denesb committed Sep 25, 2023
1 parent be942c1 commit 914cbc1
Showing 1 changed file with 3 additions and 1 deletion.
4 changes: 3 additions & 1 deletion reader_concurrency_semaphore.cc
Expand Up @@ -1121,8 +1121,10 @@ future<> reader_concurrency_semaphore::stop() noexcept {
_stopped = true;
co_await stop_ext_pre();
clear_inactive_reads();
co_await _close_readers_gate.close();
co_await _permit_gate.close();
// Gate for closing readers is only closed after waiting for all reads, as the evictable
// readers might take the inactive registration path and find the gate closed.
co_await _close_readers_gate.close();
_ready_list_cv.broken(std::make_exception_ptr(stop_execution_loop{}));
if (_execution_loop_future) {
co_await std::move(*_execution_loop_future);
Expand Down

0 comments on commit 914cbc1

Please sign in to comment.