Move Non-Read/Write Operations to Dedicated Reactor

[Besroy]

Found a deadlock issue in SH test (more details in SH issue#88):

1. Thread 1 (nuraft-reconfigure): During the replace_member process, after removing the old member, nuraft acquires the nuraft lock to trigger a reconfiguration and clears the snapshot_sync_ctx. The cleanup operation requires the current user_snp_ctx to stop, which in turn depends on all pending prefetch blobs being read. However, this operation is blocked, waiting for an I/O reactor to handle the read.

2. Thread 2 (IO reactor worker 1): This thread calls monitor_replace_member_replication_status, detects that the replace member task is completed, and attempts to reset the quorum size. However, it is blocked waiting for the nuraft lock, which is held by Thread 1. At the same time, Thread 2 holds the m_rd_map_mtx mutex.

3. Thread 3 (IO reactor worker 2): This thread calls gc_repl_reqs, which attempts to acquire the m_rd_map_mtx mutex held by Thread 2. As a result, Thread 3 is blocked.

Since both I/O reactor threads (Thread 2 and Thread 3) are blocked, no I/O operations can proceed. This prevents Thread 1 from completing the read operation required to release the nuraft lock, leading to a deadlock.

Since monitor_replace_member_replication_status and gc_repl_reqs are not typical write/read operations, should we consider isolating them from the default IOMgr workers? Below are the timers currently using default IOMgr workers:

m_rdev_gc_timer_hdl: Triggers gc_repl_reqs and gc_repl_devs every minute.
m_rdev_fetch_timer_hdl: Triggers fetch_pending_data every second.
m_flush_durable_commit_timer_hdl: Triggers flush_durable_commit_lsn every 500ms.
m_replace_member_sync_check_timer_hdl: Triggers monitor_replace_member_replication_status every minute.
m_res_audit_timer_hdl: Triggers trigger_truncate every 2 minutes.

[xiaoxichen]

how moving monitor_replace_member_replication_status away from default IOMgr worker solves the problem?

The issue is thread 1 and thread 2 generated deadlock as
thread1 holding nuraft lock and implicitly requiring m_rd_map_mtx
thread 2 is holding m_rd_map_mtx and requiring nuraft lock.

I think we probably should fix the monitor_replace_member_replication_status or HSHomeObject::PGBlobIterator::stop to break the circle

[Besroy]

how moving monitor_replace_member_replication_status away from default IOMgr worker solves the problem?

The issue is thread 1 and thread 2 generated deadlock as thread1 holding nuraft lock and implicitly requiring m_rd_map_mtx thread 2 is holding m_rd_map_mtx and requiring nuraft lock.

I think we probably should fix the monitor_replace_member_replication_status or HSHomeObject::PGBlobIterator::stop to break the circle

The issue is that thread 1 holds nuraft_lock and waits for an IO reactor to complete async_read. However, Reactor 1 holds m_rd_map_mtx while waiting for nuraft_lock, and Reactor 2 is waiting for m_rd_map_mtx. So no IO reactor can execute the async_read. Move monitor_replace_member_replication_status away from the default IOMgr and let IO reactors only handle R/W operations. This allows Thread 1 to proceed without blocking, release nuraft_lock, then Reactor 1 can acquire nuraft_lock and release m_rd_map_mtx, allowing Reactor 2 to proceed.
By the way, if we could fix monitor_replace_member_replication_status or HSHomeObject::PGBlobIterator::stop to break the circular dependency directly, that would be preferable.

[Besroy]

With AI assistance, we identified that there are many possible combinations that could lead to deadlocks. The core issue lies in management operations requiring locks being executed on IO reactor threads, which may block while waiting for other threads holding these locks to complete IO operations. The cleanest and most straightforward solution seems to be moving these operations to a dedicated thread pool instead of sharing them with the IO reactor. However, if someone is unaware of this risk, they might introduce new risky code (e.g., adding scheduled tasks back to IO workers).
Let’s document this issue and the isolation-based solution, but there’s no urgency to address it now. We can revisit, evaluate, and implement it the next time we encounter the problem. cc @koujl