Skip to content

test(sharding): add daemon shutdown deadlock test - #4619

Closed
donhardman wants to merge 15 commits into
mainfrom
fix/deadlock
Closed

test(sharding): add daemon shutdown deadlock test#4619
donhardman wants to merge 15 commits into
mainfrom
fix/deadlock

Conversation

@donhardman

@donhardman donhardman commented Jun 8, 2026

Copy link
Copy Markdown
Member

The problem we fix in this PR is: after a user creates a sharded table in a cluster consisting of multiple nodes and tries to restart one node, it may fail to restart.

@donhardman
donhardman requested a review from tomatolog June 8, 2026 16:08
@donhardman

donhardman commented Jun 8, 2026

Copy link
Copy Markdown
Member Author

Script to reproduce deadlock:

script.sh
#!/bin/bash
# ============================================================================
# Reproducer: searchd shutdown DEADLOCK on graceful `--stopwait` during a
# sharded-table rebalance on node rejoin.
#
# The daemon's main thread enters Shutdown() -> Threads::ThreadPool_c::StopAll()
# and blocks FOREVER joining a worker thread that was never signalled to exit.
# `searchd --stopwait` therefore never returns. 100% daemon-side (no Buddy).
#
# HOW TO RUN (on a host with docker):
#   IMG=ghcr.io/manticoresoftware/manticoresearch:test-kit-bdf38ac
#   docker run --rm -i --cap-add=SYS_PTRACE \
#     -v /path/to/manticoresearch:/work -w /work \
#     "$IMG" bash -s < reproduce_deadlock.sh
#
#   (/work only needs test/clt-tests/base/searchd-with-flexible-ports.conf;
#    any manticoresearch checkout works.)
#
# EXIT 1 + "DEADLOCK CONFIRMED" => bug reproduced (node2 --stopwait hung).
# EXIT 0 + "no deadlock"        => daemon stopped gracefully (fixed/not repro).
# ============================================================================
set -u
CONF=test/clt-tests/base/searchd-with-flexible-ports.conf

start_node() {       # $1 = instance number
  local I=$1; export INSTANCE=$I
  mkdir -p /var/run/manticore-$I /var/lib/manticore-$I /var/log/manticore-$I
  # --logreplication only sharpens timing; the deadlock does not depend on it.
  stdbuf -oL searchd --logreplication -c "$CONF" >/dev/null 2>&1
  timeout 40 grep -qm1 '\[BUDDY\] started' \
    <(tail -n 1000 -f /var/log/manticore-$I/searchd.log 2>/dev/null) \
    || { echo "FATAL: node$I failed to start"; exit 2; }
}
is_primary() { mysql -h0 -P"$1" -e "SHOW STATUS LIKE 'cluster_c_status'\G" 2>/dev/null | grep -q "Value: primary"; }
poll_primary() { local d=$((SECONDS+60)); while [ $SECONDS -lt $d ]; do
  local ok=1; for n in "$@"; do is_primary "${n}306" || ok=0; done; [ $ok = 1 ] && return 0; sleep 0.5; done; return 1; }

echo "### 1) start 3-node cluster, create RF=2 sharded table, insert"
start_node 1; start_node 2; start_node 3
mysql -h0 -P1306 -e "create cluster c" >/dev/null 2>&1
mysql -h0 -P2306 -e "join cluster c at '127.0.0.1:1312'" >/dev/null 2>&1
mysql -h0 -P3306 -e "join cluster c at '127.0.0.1:1312'" >/dev/null 2>&1
poll_primary 1 2 3 || { echo "FATAL: cluster not primary after create"; exit 2; }
mysql -h0 -P1306 -e "CREATE TABLE c:t (id bigint, account string, amount float, ts int) shards='2' rf='2'" >/dev/null 2>&1
mysql -h0 -P1306 -e "INSERT INTO t (id,account,amount,ts) VALUES (1,'A',1,1),(2,'B',2,2),(3,'C',3,3),(4,'A',4,4),(5,'B',5,5)" >/dev/null 2>&1

echo "### 2) first failure: stop node1 (this graceful stop WORKS), restart it"
export INSTANCE=1; searchd --stopwait -c "$CONF" >/dev/null 2>&1
timeout 30 bash -c 'while lsof -i :1306 &>/dev/null; do sleep 0.2; done'
timeout 15 grep -qm1 'becoming master' <(tail -n 1000 -f /var/log/manticore-2/searchd.log /var/log/manticore-3/searchd.log 2>/dev/null)
mysql -h0 -P2306 -e "INSERT INTO t (id,account,amount,ts) VALUES (6,'C',6,6),(7,'A',7,7)" >/dev/null 2>&1
start_node 1
poll_primary 1 2 3 || echo "WARN: not all primary after node1 rejoin"

echo "### 3) second failure: graceful-stop node2 during the sharded rebalance"
echo "###    healthy daemon returns in <few s; buggy daemon DEADLOCKS in StopAll()"
export INSTANCE=2
pid=$(cat /var/log/manticore-2/searchd.pid 2>/dev/null)
t0=$SECONDS
timeout 45 searchd --stopwait -c "$CONF" >/dev/null 2>&1
rc=$?
echo "node2 --stopwait exit=$rc  (elapsed=$((SECONDS-t0))s)  [124 = timed out = DEADLOCK]"

if [ "$rc" = 124 ]; then
  echo
  echo "==================== DEADLOCK CONFIRMED ===================="
  echo "node2 searchd.log last line (where shutdown stalls):"
  tail -1 /var/log/manticore-2/searchd.log 2>/dev/null
  echo
  echo "--- backtrace of the hung daemon (pid $pid) ---"
  if ! command -v gdb >/dev/null; then
    apt-get update -qq >/dev/null 2>&1 && DEBIAN_FRONTEND=noninteractive apt-get install -y -qq gdb >/dev/null 2>&1
  fi
  if command -v gdb >/dev/null && [ -n "$pid" ]; then
    gdb -p "$pid" -batch -ex "set pagination off" \
        -ex "thread apply all bt" 2>/dev/null \
      | grep -E "^Thread |StopAll|Shutdown|clockjoin|pthread_join|do_run_one|ThreadPool_c::loop|ServiceThd|ServiceMain|TickHead|CheckSignals|^#4|^#5" \
      | head -60
    mkdir -p /cap 2>/dev/null && gdb -p "$pid" -batch -ex "set pagination off" -ex "thread apply all bt" 2>/dev/null > /cap/deadlock_bt.txt && chmod 777 /cap/deadlock_bt.txt 2>/dev/null
    echo "(full backtrace saved to /cap/deadlock_bt.txt if /cap mounted)"
  else
    echo "(gdb unavailable; rerun with --cap-add=SYS_PTRACE and network for apt)"
  fi
  pkill -9 searchd 2>/dev/null
  exit 1
else
  echo "no deadlock: node2 stopped gracefully (exit $rc)"
  pkill -9 searchd 2>/dev/null
  exit 0
fi

@donhardman

Copy link
Copy Markdown
Member Author

I see some unit test fails, while the fix fixing the CLT test. The script to reproduce attached. @klirichek can you look into it, is it valid fix?

@donhardman
donhardman requested review from klirichek and removed request for tomatolog June 8, 2026 16:31
@manticoresoftware manticoresoftware deleted a comment from github-actions Bot Aug 14, 2026
@manticoresoftware manticoresoftware deleted a comment from github-actions Bot Aug 14, 2026
@manticoresoftware manticoresoftware deleted a comment from github-actions Bot Aug 14, 2026
@manticoresoftware manticoresoftware deleted a comment from github-actions Bot Aug 14, 2026
@manticoresoftware manticoresoftware deleted a comment from github-actions Bot Aug 14, 2026
After fix of #3905, sockets data (connection) was left as is, but socket
itself was already closed. This record was removed by timeout. Now it
removed right on the next tick of netpool. It makes other visible things
 (like increasing mem consumption by connections) non actual.
It is implied, there are no dupes in usual replication workflow.
However at edge cases it should be temporary allowed.
This is fine-tune, ruled by MANTICORE_SHUTDOWN_ALONES_DEADLINE and
MANTICORE_SHUTDOWN_ALONES_POLL env.
@sanikolaev

Copy link
Copy Markdown
Collaborator

The original CLT test is still failing. We need to figure out why and either update the test or fix the code.

klirichek and others added 7 commits August 19, 2026 18:04
ThreadPool_c::StopAll() already force-stops the scheduler on fatal
shutdown. Graceful SIGTERM needs the same treatment: sphInterrupted()
means the daemon is shutting down and no longer wants to drain leftover
scheduler keepers.

Without this, a replication/Buddy rebalance can leave outstanding work
while all worker threads are idle on an empty queue, making stopwait block
forever in pthread_join(). Stopping the service during SIGTERM wakes those
threads and lets shutdown complete.
Keep the default StopAll behavior draining, and make daemon shutdown explicitly abort tick and global pools after its bounded client drain. Stop the idle RT merge worker once clients are gone, and report outstanding work and queued tasks when the abort fallback is needed.
Remove the pre-release RT merge-worker stop that could hang Windows daemon restarts before RT state was persisted. Keep explicit work-pool abort as the bounded shutdown fallback.
klirichek added a commit that referenced this pull request Aug 24, 2026
Replaces pr 4619 (cleaned from supplement code not related to the fix)

ref: #4619
ref: #3905
@klirichek

Copy link
Copy Markdown
Contributor

only relevant commits left in #4829

@klirichek klirichek closed this Aug 24, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants