Skip to content

Fix hangs in CI builds - #940

Merged
nnastonen merged 4 commits into
eBay:dev/v8.xfrom
nnastonen:fix_raft_fetch_after_leave_group
Oct 27, 2026
Merged

nnastonen merged 4 commits into
eBay:dev/v8.xfrom
nnastonen:fix_raft_fetch_after_leave_group

Conversation

@nnastonen

@nnastonen nnastonen commented Oct 20, 2026 •

Copy link
Copy Markdown
Contributor

Since _PRERELEASE has been enabled, it exposed problems with replicas and synchronisation between them. This is an attempt to fix them.

Why

The Debug CI jobs (ASan, TSan, Coverage) intermittently hung until GitHub killed them after 6 hours, or crashed, with nothing in the log to diagnose. Release builds passed. The causes were real races in HomeStore plus bugs in tests and the test harness. Some were hidden because the "ASan" job wasn't actually built with ASan.

Changes

Product fixes

  • RaftReplDev: skip data fetches for a repl dev that is no longer active, and drain in-flight fetches before leave_group() frees the raft server. A late fetch crashed the follower, and the other replicas waited forever;
  • BlkReadTracker: guard the waiter list. wait_on() modified it under the bucket's shared lock only;
  • home_log_store::truncate(): serialize concurrent truncates of a store;
  • JournalVirtualDev: lock the descriptor map in open(), destroy() and load;
  • CPWatchdog: it never ran (!= a || != b), and its checks were inverted. It now crashes the node on a stuck CP.

Tests

  • Raft repl harness:
  • Waits on other replicas are bounded (--wait_timeout_sec, default 600s) and report what they were waiting for;
  • The IPC barrier is generation-based; consecutive barriers of the same phase used to let replicas through early;
  • Spare replicas are counted in the register_listener() barrier;
  • SoloReplDev: the replay check accounts for journal truncation by CPs and verifies the exact count;
  • LogStore: keep flushing while waiting for inserts. The test's explicit-flush log devices could lose a flush to a parallel truncate;
  • IndexCrashRecovery: don't trigger a CP into a simulated crash that has already started.

CI

  • The ASan job now builds with ASan and UBSan; CMake ignored ADDRESS_SANITIZER_ON before. It uses 2 parallel compile jobs to fit in runner memory;
  • Failing tests print their output, and each test has a 30-minute timeout;
  • Hung or crashed tests print all thread stacks (stack_dump_launcher.sh with gdb), and test logs are uploaded as an artifact.

SDSTOR-26170

@nnastonen
nnastonen force-pushed the fix_raft_fetch_after_leave_group branch from 763e499 to d19e2ec Compare October 20, 2026 14:11
@codecov-commenter

codecov-commenter commented Oct 20, 2026 •

Copy link
Copy Markdown

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

❌ Patch coverage is 20.83333% with 38 lines in your changes missing coverage. Please review.
⚠️ Please upload report for BASE (dev/v8.x@1fc342e). Learn more about missing BASE report.

Files with missing lines Patch % Lines
src/lib/checkpoint/cp_mgr.cpp 13.63% 19 Missing ⚠️
src/lib/blkdata_svc/blk_read_tracker.cpp 0.00% 0 Missing and 4 partials ⚠️
src/lib/device/journal_vdev.cpp 0.00% 0 Missing and 4 partials ⚠️
src/lib/logstore/log_store.cpp 0.00% 2 Missing and 2 partials ⚠️
src/lib/replication/repl_dev/raft_repl_dev.cpp 57.14% 2 Missing and 1 partial ⚠️
src/lib/replication/service/raft_repl_service.cpp 40.00% 0 Missing and 3 partials ⚠️
src/include/homestore/replication/repl_dev.hpp 50.00% 1 Missing ⚠️
❗ Your organization needs to install the Codecov GitHub app to enable full functionality.
Additional details and impacted files
@@             Coverage Diff             @@
##             dev/v8.x     #940   +/-   ##
===========================================
  Coverage            ?   49.18%           
===========================================
  Files               ?      112           
  Lines               ?    12790           
  Branches            ?     6061           
===========================================
  Hits                ?     6291           
  Misses              ?     2468           
  Partials            ?     4031           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@nnastonen
nnastonen force-pushed the fix_raft_fetch_after_leave_group branch 2 times, most recently from 4840e87 to 06faba3 Compare October 21, 2026 16:13
@nnastonen
nnastonen marked this pull request as ready for review October 21, 2026 17:25
Comment thread src/lib/checkpoint/cp_mgr.cpp Resolved
- RaftReplDev: don't fetch data for a repl dev that is no longer active, and
  drain in-flight fetches before leave_group() frees the raft server. A late
  fetch dereferenced the freed server and crashed the follower; the other
  replicas then waited forever.
- BlkReadTracker: guard the waiter list; wait_on() appended to it under the
  bucket's shared lock only.
- home_log_store::truncate(): serialize concurrent truncates of a store.
- JournalVirtualDev: lock the descriptor map in open(), destroy() and load.
- CPWatchdog: it never checked anything, and its progress and deadline checks
  were inverted. A CP that makes no progress for 12 timer periods, with no
  consumer left to repair, now asserts with its state.
- Raft repl test harness: bound every wait on other replicas
  (--wait_timeout_sec, default 600s) and report what it waited for; make the
  IPC barrier generation based, so consecutive barriers of the same phase no
  longer let replicas through early; count spare replicas in the
  register_listener() barrier.
- SoloReplDev: CPs truncate part of the journal before the restart in slow
  runs, so expect replays only for the writes after the truncation point, and
  check that count exactly.
- LogStore: keep flushing while waiting for inserts; a flush can be skipped
  while a truncate holds the flush lock.
- IndexCrashRecovery: don't trigger a CP into an already started simulated
  crash.
- Build with AddressSanitizer when sanitize=address (CMake only read
  MEMORY_SANITIZER_ON, so the ASan job was a plain Debug build), with two
  parallel compiles to stay within the runner's memory.
- Print the output of failing tests and give every test a 30 minute timeout.
- Run tests through stack_dump_launcher.sh, which prints all thread stacks of a
  test still running after 25 minutes and of a test that crashed (from its
  core file).
- Upload the test logs as an artifact.
@nnastonen
nnastonen force-pushed the fix_raft_fetch_after_leave_group branch from 06faba3 to 1d1cc6c Compare October 23, 2026 08:57
raakella1
raakella1 previously approved these changes Oct 23, 2026

@raakella1 raakella1 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LG!

@nnastonen
nnastonen force-pushed the fix_raft_fetch_after_leave_group branch from 0b25238 to b50d30e Compare October 27, 2026 13:16

@raakella1 raakella1 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LG!

@nnastonen
nnastonen merged commit 4d93ef5 into eBay:dev/v8.x Oct 27, 2026
32 of 33 checks passed
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