fix(server): pipelined batch-tail hardening — remote-pending crash class, D4 migration gate, carried-remainder stall (#438) by TinDang97 · Pull Request #444 · pilotspace/moon

TinDang97 · 2026-08-07T08:16:40Z

Part of #438 (D4, plus three adjacent findings the D4 red test surfaced). F1–F6 + conn-secondary remain open.

What the red test found

Writing the D4 regression test uncovered that the pipelined batch tail is mishandled at every hand-off boundary, including one Critical, remotely-triggerable whole-process crash present on both runtimes:

1. Crash class (Critical). On any --shards ≥ 2 deployment, ONE pipelined write [GET <remote-key>…, BLPOP] (or SUBSCRIBE / PSUBSCRIBE / PSYNC) aborted the server: the early-flush arm flushed the remote commands' Frame::Null placeholders (wrong replies), cleared/replaced responses, and phase 2's remote-reply drain indexed out of bounds — shard-thread panic → deliberate process abort. Reproduced on pristine main with a 20-line script.

Fix: early-flush commands defer themselves + the unconsumed batch tail (re-encoded losslessly into the front of read_buf, carried_input armed) whenever remote-slotted work is pending. Phase 2 resolves and flushes every prior reply, then the deferred command runs at the head of a clean batch. The deferral fires only in the exact combination that panics today.

2. D4 — migration races the batch tail. migration_target latched mid-batch, executed at batch end unchecked; […GETs, MULTI, SET] migrated with the txn queued → discarded → -ERR EXEC without MULTI. New ConnectionState::migration_eligible() (¬MULTI, ¬cross-txn, ¬subs, ¬tracking, ¬replica — last two are new coverage) evaluated at both latch and batch-end execution; an ineligible batch end keeps the latch and migrates at the first clean one.

3. Migrated-connection remainder stall. A resumed migrated handler received read_buf_remainder but initialized carried_input = false — a pipelined tail crossing a migration sat unanswered until the client sent more bytes. Now arms on non-empty initial buffer; subscriber loops gained a carry-aware read arm.

4. Swallowed batch tail. [SUBSCRIBE ch, PING] in one write dropped the PING (frames.drain(..) discarded the remainder on break). Frame loops are index-based; Subscribed/blocking breaks carry the tail forward.

Red/green

tests/migration_batch_tail.rs (4 tests) + migration_eligibility_gate unit test:

RED pristine main: tokio-macOS 0/4 (tokio migrates on macOS — real D4 exercise), monoio-macOS 1/4 (no migration off-Linux, crash legs still fail)
GREEN with fix: 4/4 monoio-macOS, 4/4 tokio-macOS, 4/4 monoio-Linux (moon-dev VM — real fd-passing migration)
The MULTI leg is placement-deterministic: two phases on one connection (shard-0 keys, then shard-1 keys past the 64-cmd re-migration trigger) guarantee one phase converges on a remote shard.

Gates

clippy -D warnings both feature sets; fmt
Full monoio release suite green (one oom_bypass_closure env flake under parallel load, 8/8 in isolation); tokio lib 3653/3653
Bench waived with rationale: frame-loop restructure is mem::replace-per-frame vs Drain::next (same cost); the new guard is two Vec::is_empty loads per frame, name compares only while the batch already has pending cross-shard work (the SPSC hop dominates by ~3 orders of magnitude)

Refs #438

Summary by CodeRabbit

Bug Fixes
- Prevented crashes and lost commands when pipelined requests include blocking or subscription commands.
- Preserved command ordering and pending requests during connection migration.
- Improved handling of buffered data after migration and when entering subscriber mode.
- Prevented unsafe migration during transactions, subscriptions, tracking, and replica handshakes.
- Fixed stalled migrated connections and swallowed commands after blocking or subscription operations.
Tests
- Added coverage for migration, pipelined command tails, transactions, subscriptions, and blocking commands.

…ass, D4 migration gate, carried-remainder stall (#438) Writing the red test for #438 D4 (migration races the pipelined batch tail) surfaced three adjacent pre-existing defects in the same frame-loop neighborhood; all four are fixed together because they share one mechanism (the batch tail crossing an early-flush or hand-off boundary) and one test file. 1) Remotely-triggerable whole-process crash (both runtimes, Critical). On --shards >= 2, ONE pipelined write `[GET <remote-key>…, BLPOP]` (or SUBSCRIBE/PSUBSCRIBE/PSYNC) aborted the server: the early-flush arm flushed the remote commands' Frame::Null placeholders (wrong replies), cleared/replaced `responses`, and phase 2's remote-reply drain then indexed out of bounds — shard-thread panic, process abort. Reproduced on pristine main with a 20-line python script; both handlers affected. Fix: early-flush commands defer themselves plus the unconsumed batch tail (re-encoded losslessly into the front of read_buf, carried_input armed) whenever remote-slotted work is pending; phase 2 resolves and the epilogue flushes every prior reply first, then the deferred command runs at the head of a clean batch. The deferral triggers only in the exact combination that panics today — no behavior change for any working workload; local-only batches pay two Vec::is_empty loads. 2) D4: migration gate re-evaluated at the execution point. The affinity sampler latched migration_target mid-batch with a gate check at latch time only; `[…GETs, MULTI, SET]` migrated with the transaction queued, and MigratedConnectionState carries no command_queue/in_multi/subs/tracking — queued txn discarded, EXEC answered "-ERR EXEC without MULTI", tail SUBSCRIBE orphaned. New ConnectionState::migration_eligible() (not in MULTI, no cross-store txn, no subscriptions, no CLIENT TRACKING, not a replica — the last two are new coverage) is evaluated at BOTH the latch and the batch-end execution point; an ineligible batch end keeps the latch armed and the migration runs at the first clean batch end (e.g. right after EXEC). 3) Migrated connections stalled on their carried remainder. A resumed migrated handler received read_buf_remainder but initialized carried_input=false, so its first select awaited a fresh socket read — a pipelined tail crossing a migration sat unanswered until the client sent more bytes. carried_input now initializes to !read_buf.is_empty() (both handlers), and the subscriber-mode loops gained a carry-aware read arm so a deferred tail reaches them without awaiting the socket. 4) Parsed batch tail after SUBSCRIBE/blocking was silently dropped. `[SUBSCRIBE ch, PING]` in one write swallowed the PING (frame iterator discarded the remainder on break). The frame loops are now index-based and any Subscribed/blocking break carries the unconsumed tail forward. Tests (red/green, tests/migration_batch_tail.rs): - multi_batch_tail_survives_migration_convergence — two-phase single-conn design (shard-0 keys then shard-1 keys with the 64-cmd re-migration trigger) guarantees one phase converges on a REMOTE shard regardless of placement; EXEC-exactness asserted both phases. - blocking_tail_with_pending_remote_does_not_crash — the crash-class repro, both key sets, plus liveness PING. - subscribe_batch_tail_survives_migration_convergence — subscription survives the converging batch and still receives PUBLISH. - subscribe_tail_frames_are_not_swallowed — [SUBSCRIBE, PING] one write. - Unit: migration_eligibility_gate pins every blocker + restoration. RED on pristine main: tokio 4/4 fail (incl. the D4 MULTI leg — tokio migrates on macOS), monoio-macOS 3/4 fail (no migration off-Linux). GREEN with fix: 4/4 both runtimes; monoio-Linux leg run on moon-dev VM. Gates: clippy -D warnings both feature sets; fmt; full monoio release suite; tokio suite. Bench waived with rationale: the frame loop's restructure is mem::replace-per-frame instead of Drain::next (same cost), and the new guard is two Vec::is_empty loads per frame, name compares only while a batch already has pending cross-shard work (SPSC hop dominates by ~3 orders of magnitude). Refs #438 (D4 + new findings; F1-F6 + conn-secondary remain open) author: Tin Dang

coderabbitai · 2026-08-07T08:17:52Z

Warning

Review limit reached

@TinDang97, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 49 minutes

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: b5cced1a-6788-4175-b5dc-65b678e1c59b

📥 Commits

Reviewing files that changed from the base of the PR and between ea3e2ff and 02bd853.

📒 Files selected for processing (3)

src/server/conn/handler_monoio/mod.rs
src/server/conn/handler_sharded/mod.rs
src/server/conn/handler_sharded/pubsub.rs

📝 Walkthrough

Walkthrough

The connection handlers now preserve pipelined command tails across blocking, subscription, and migration transitions. They centralize blocking-command detection, validate migration eligibility, process carried input immediately, and add regression coverage for migration and batch-tail behavior.

Changes

Connection pipeline and migration handling

Layer / File(s)	Summary
Shared command and migration gates `src/server/conn/blocking.rs`, `src/server/conn/core.rs`, `src/server/conn/handler_monoio/dispatch.rs`, `src/server/conn/tests.rs`	Centralizes blocking-command detection and adds migration eligibility checks for transaction, subscription, tracking, cross-store, and replica state.
Monoio carry and deferred-tail processing `src/server/conn/handler_monoio/mod.rs`	Preserves deferred frames, processes carried input without a socket read, re-encodes pipeline tails, and rechecks migration eligibility before handoff.
Sharded carry, subscription, and migration flow `src/server/conn/handler_sharded/mod.rs`, `src/server/conn/handler_sharded/pubsub.rs`	Preserves frames after blocking or subscription commands, parses carried subscriber input, and validates migration eligibility at latch and batch completion.
Migration and batch-tail validation `tests/migration_batch_tail.rs`, `CHANGELOG.md`	Adds integration coverage for migration, subscriptions, transactions, blocking commands, and commands following `SUBSCRIBE`. Documents the fixes.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Client
  participant ConnectionHandler
  participant RemoteReplies
  participant ReadBuffer
  participant SubscriberMode
  Client->>ConnectionHandler: Send pipelined commands
  ConnectionHandler->>RemoteReplies: Dispatch pending remote work
  ConnectionHandler->>SubscriberMode: Enter subscription handling
  SubscriberMode->>ReadBuffer: Preserve unread command tail
  RemoteReplies-->>ConnectionHandler: Resolve pending replies
  ConnectionHandler->>ReadBuffer: Re-encode deferred frames
  ConnectionHandler->>Client: Process carried commands in order

Possibly related PRs

pilotspace/moon#213: Both modify connection-handler batch and early-flush logic.
pilotspace/moon#230: Both modify connection migration handling.
pilotspace/moon#278: Both modify PSYNC and migration or replication state handling.

Suggested reviewers: pilotspacex-byte

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly identifies the pipelined batch-tail hardening and its main crash, migration, and carried-input fixes.
Description check	✅ Passed	The description explains the problem, fixes, tests, quality gates, and performance rationale, although it does not use the template headings exactly.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

📝 Generate docstrings

Create stacked PR
Commit on current branch

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch fix/438-d4-migration-batch-tail

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands.}

qodo-code-review · 2026-08-07T08:18:27Z

PR Summary by Qodo

Harden pipelined batch tails: remote-pending crash, D4 migration gate, carry fixes

🐞 Bug fix 🧪 Tests 📝 Documentation 🕐 40+ Minutes

AI Description

• Prevent early-flush commands from crashing when remote-slotted replies are pending.
• Gate connection migration on transferable state, re-checked at migration execution.
• Carry pipelined batch tails across SUBSCRIBE/blocking breaks and migrations; add regression tests.

Diagram

graph TD
  A([Client]) --> B["Conn handler (sharded)"] --> C["RESP decode + batch"] --> D["Frame loop (index)"] --> E["Phase-2 remote drains"] --> F["Flush responses"]
  D --> G["Defer tail into read_buf"]
  D --> H["Migration eligibility gate"]
  D --> I["Subscriber/blocking early-flush"]

High-Level Assessment

The following are alternative approaches to this PR:

1. Keep deferred tail as parsed frames (no re-encode)

➕ Avoids RESP re-serialization and re-parsing overhead
➕ Can preserve original frame allocations
➖ Harder to carry across handler hand-off (migration passes bytes, not frame vectors)
➖ More complex lifetime/ownership across loop iterations and mode switches (subscriber/blocking)

2. Disallow early-flush inside a batch when any remote work exists

➕ Conceptually simpler: enforce a strict ‘remote first’ rule
➕ Reduces deferral machinery
➖ Changes latency/behavior for blocking/pubsub commands; could delay expected early replies
➖ Still needs a strategy for already-parsed tail frames when breaking into subscriber/blocking mode

Recommendation: The chosen approach (conditionally deferring the early-flush command and re-encoding the unconsumed tail back into read_buf) is the best fit because it unifies all hand-off boundaries (blocking break, SUBSCRIBE break, and migration hand-off) around a single representation: bytes. That makes the fix robust across both runtimes and avoids introducing new cross-iteration frame ownership complexity. The additional migration_eligible gate, re-checked at batch-end execution, is the correct place to prevent silent state loss.

Files changed (9) +751 / -33

Bug fix (4) +218 / -24

core.rsAdd ConnectionState::migration_eligible() gate +20/-0

Add ConnectionState::migration_eligible() gate

• Defines a single predicate for whether migration can safely occur, blocking migration while in MULTI, cross-store transactions, subscriptions, CLIENT TRACKING, or replica handshake state.

src/server/conn/core.rs

mod.rsDefer early-flush tails, carry parsed remainder, re-check migration at batch end (monoio) +99/-10

Defer early-flush tails, carry parsed remainder, re-check migration at batch end (monoio)

• Arms carried_input when starting with a non-empty read buffer (migrated/resumed connections). Converts frame processing to index-based iteration so SUBSCRIBE/blocking breaks and early-flush commands can defer themselves plus the unconsumed tail by re-encoding it into read_buf. Adds a remote-pending + early-flush deferral guard to prevent flushing placeholder replies and crashing, and re-checks migration eligibility at execution time.

src/server/conn/handler_monoio/mod.rs

mod.rsMirror batch-tail deferral/carry and migration gate in tokio sharded handler +84/-13

Mirror batch-tail deferral/carry and migration gate in tokio sharded handler

• Arms carried_input for migrated connections, threads a have_carry signal into subscriber mode, switches to index-based batch iteration, carries parsed tails across breaks, re-encodes deferred tails into read_buf, and re-checks migration eligibility at batch end.

src/server/conn/handler_sharded/mod.rs

pubsub.rsParse carried tail in subscriber mode without awaiting socket read +15/-1

Parse carried tail in subscriber mode without awaiting socket read
• Adds a have_carry parameter and uses an in-band CARRY_READY marker to run the same parse loop when bytes are already present in read_buf from a deferred tail.
src/server/conn/handler_sharded/pubsub.rs

Refactor (2) +16 / -9

blocking.rsCentralize blocking-command detection +15/-0

Centralize blocking-command detection

• Introduces is_blocking_command() enumerating all early-flush blocking commands, intended to be kept in sync with dispatch guards across handlers.

src/server/conn/blocking.rs

dispatch.rsUse shared blocking-command predicate in monoio dispatch +1/-9

Use shared blocking-command predicate in monoio dispatch
• Replaces duplicated command-name checks with blocking::is_blocking_command() to keep behavior consistent with other handlers.
src/server/conn/handler_monoio/dispatch.rs

Tests (2) +486 / -0

tests.rsUnit test for migration eligibility predicate +65/-0

Unit test for migration eligibility predicate

• Adds migration_eligibility_gate test verifying that non-transferable states (MULTI, subs, cross-txn, tracking, replica handshake) block migration and that clearing state restores eligibility.

src/server/conn/tests.rs

migration_batch_tail.rsRegression tests for migration vs pipelined batch tails and early-flush crash class +421/-0

Regression tests for migration vs pipelined batch tails and early-flush crash class

• Adds an integration test suite that spawns a multi-shard server and exercises: MULTI queued in a batch tail during migration convergence, blocking tail with pending remote work (no crash), SUBSCRIBE tail not swallowing subsequent frames, and subscription survival across migration convergence.

tests/migration_batch_tail.rs

Documentation (1) +31 / -0

CHANGELOG.mdDocument #438 follow-on fixes for batch-tail crash/migration/stalls +31/-0

Document #438 follow-on fixes for batch-tail crash/migration/stalls

• Adds detailed release notes describing the remotely-triggerable crash class, the D4 migration eligibility re-check, the migrated-connection remainder stall fix, and the SUBSCRIBE/blocking tail carry behavior.

CHANGELOG.md

coderabbitai

Actionable comments posted: 1

🧹 Nitpick comments (1)

tests/migration_batch_tail.rs (1)
357-374: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Consider covering the pubsub-arm race in this test.

subscribe_tail_frames_are_not_swallowed sends [SUBSCRIBE ch, PING] with no message pending on the channel, so the subscriber-mode read arm always wins the select and the carry is consumed correctly. The failure mode flagged at src/server/conn/handler_sharded/mod.rs line 450 and src/server/conn/handler_monoio/mod.rs line 520 needs a message queued on the channel at the moment the handler enters subscriber mode.

A second connection that publishes to ch immediately before the pipelined [SUBSCRIBE ch, PING] write would make the race reachable. The test would be timing-dependent, so gate the assertion on a generous deadline rather than a single read.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/migration_batch_tail.rs` around lines 357 - 374, Update
subscribe_tail_frames_are_not_swallowed to use a second connection that
publishes to migch:t immediately before the tested client sends the pipelined
SUBSCRIBE and PING commands, ensuring a channel message is queued when
subscriber mode starts. Replace the single immediate reply read with
deadline-bounded polling or reads, while preserving assertions that both the
subscription confirmation and PING response arrive.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/server/conn/handler_monoio/mod.rs`:
- Around line 514-528: Preserve carried_input until the read arm actually
consumes it: in src/server/conn/handler_monoio/mod.rs lines 514-528, read the
flag without taking it before monoio::select!, then clear carried_input inside
the read_result arm; in src/server/conn/handler_sharded/mod.rs lines 447-450,
pass &mut carried_input to run_subscriber_step and clear it inside that
function’s tokio::select! read arm.

---

Nitpick comments:
In `@tests/migration_batch_tail.rs`:
- Around line 357-374: Update subscribe_tail_frames_are_not_swallowed to use a
second connection that publishes to migch:t immediately before the tested client
sends the pipelined SUBSCRIBE and PING commands, ensuring a channel message is
queued when subscriber mode starts. Replace the single immediate reply read with
deadline-bounded polling or reads, while preserving assertions that both the
subscription confirmation and PING response arrive.

🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

Push a commit to this branch (recommended)
Create a new PR with the fixes

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 14be1fad-a67d-44eb-919f-93442a45c25f

📥 Commits

Reviewing files that changed from the base of the PR and between c0ba6fb and ea3e2ff.

📒 Files selected for processing (9)

CHANGELOG.md
src/server/conn/blocking.rs
src/server/conn/core.rs
src/server/conn/handler_monoio/dispatch.rs
src/server/conn/handler_monoio/mod.rs
src/server/conn/handler_sharded/mod.rs
src/server/conn/handler_sharded/pubsub.rs
src/server/conn/tests.rs
tests/migration_batch_tail.rs

qodo-code-review · 2026-08-07T08:24:35Z

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📜 Skill insights (0)

1. ~~Migration code not Linux-gated~~ ✗ Dismissed 📘 Rule violation ≡ Correctness

Description

New connection-migration logic is compiled unconditionally (not behind `#[cfg(target_os =
"linux")]`), so migration behavior remains available on non-Linux targets. This violates the
requirement that connection migration be Linux-only and risks unsupported platform behavior (e.g.,
macOS).

Code

src/server/conn/core.rs[R459-462]

+    pub fn migration_eligible(&self) -> bool {
+        !self.in_multi
+            && self.active_cross_txn.is_none()
+            && self.subscription_count == 0

Relevance

●●● Strong
Repo previously gated migration behavior for non-unix; Linux-only gating aligns with established
platform-guard pattern.
PR-#147

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
PR Compliance ID 351131 requires connection migration code paths/APIs to be gated to `target_os =
"linux". The PR introduces ConnectionState::migration_eligible()` and uses it to execute
migrations, but neither the new function nor the migration execution check is Linux-gated.
Rule 351131: Gate connection migration to Linux only
src/server/conn/core.rs[447-465]
src/server/conn/handler_sharded/mod.rs[2467-2476]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Connection migration functionality must be Linux-only, but the PR adds/uses migration-specific logic without `#[cfg(target_os = "linux")]` gating.

## Issue Context
Compliance requires that any APIs/paths enabling or implementing connection migration are compiled only for Linux. The new `ConnectionState::migration_eligible()` and its call sites are part of the migration mechanism and currently compile on non-Linux platforms.

## Fix Focus Areas
- src/server/conn/core.rs[447-465]
- src/server/conn/handler_sharded/mod.rs[2467-2476]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Context used

✅ Compliance rules (platform): 55 rules

_{To customize comments, go to the Qodo configuration screen, or learn more in the docs.}

…ins (#438 review) CodeRabbit (PR #444): both subscriber-mode selects consumed carried_input with mem::take BEFORE the select resolved — a round won by the pubsub or shutdown arm ate the flag with the deferred tail still unparsed, re-creating the stall the PR fixes. The flag is now read (not taken) when computing have_carry and cleared in the read arm's BODY, which runs only when that arm wins; a lost round keeps the carry armed and retries. Applied to the monoio subscriber select, the sharded run_subscriber_step (now takes &mut carried_input), and the pre-existing take in the sharded main select, which had the same race against its tracking-push arm. Refs #438 author: Tin Dang

…iene) (#449) Pid-only temp-dir names (`temp_dir().join(format!("...-{}", pid))`) at eight spawn sites across six suites resurrected STALE data dirs: a crashed run leaves its dir behind, the pid is eventually reused, and the next run's server silently reloads the leftover persistence state (the documented CWD/stale-reload trap) — failures then point at whatever assertion tripped over the ghost data, not at the cause. - info_memory_allocator_pagecache, memory_doctor_response, memory_prometheus_kinds: the shared Moon harness now owns a tempfile::TempDir (random unique name; removed on drop AFTER the child is killed — hand-written Drop body runs before field drops). - vector_exact_rerank (3 sites): RAII tempdir; also removes the manual pre/post remove_dir_all pairs, so cleanup now happens even when an assert panics mid-test. - vector_db_isolation (2 sites): unique random dirs via tempfile::Builder + keep(). Deliberately NOT RAII: the restart test shares one dir between two Moon values (kill_keep_dir → same-port respawn), so the existing PathBuf ownership + manual cleanup are kept and only the collision-prone NAME is fixed. - tls_park_keyupdate: RAII tempdir; early-return and end-of-test manual removes deleted. parked_idle_parity deadline-poll fixes (the flakes deferred from the #444/#445 rounds, all environmental classes observed in CI or today's gate runs): - CLIENT KILL registry-removal assert now POLLS with a 10s deadline — the client-side close arrives instantly via shutdown(2) but the registry entry is released only when the killed handler task gets scheduled; the read-once assert fired 3/3 on the starved 2-vCPU runner and passed solo (documented assert-too-soon race). - All 13 TcpStream::connect sites use a connect_retry helper (10s deadline, 50ms backoff): under full-suite load a freshly listening server can still refuse the first attempts, which failed tests at the connect line (seen live today at the F6 test's connect). - Read deadlines 10s → 30s in the two reply helpers; deadline-bound, so green runs spend no extra time. The client_tracking_invalidation multikey second-key push flake is PRODUCT-side (a real RESP3 client would miss the same invalidation) — filed as #448 with the merge-base A/B evidence instead of being papered over here. Gates: fmt + clippy -D warnings (default and tokio,jemalloc); all six converted suites + parked_idle_parity green on macOS monoio (31 tests); tokio leg green on the four suites that run there. Bench gates waived — tests-only change, no src/ code touched. Refs #444 #445 #446, closes nothing (#448 filed for the product flake) author: Tin Dang

coderabbitai Bot reviewed Aug 7, 2026

View reviewed changes

Comment thread src/server/conn/handler_monoio/mod.rs

qodo-code-review Bot reviewed Aug 7, 2026

View reviewed changes

Comment thread src/server/conn/core.rs

TinDang97 merged commit 8e2fe8f into main Aug 7, 2026
18 of 19 checks passed

TinDang97 deleted the fix/438-d4-migration-batch-tail branch August 7, 2026 11:05

TinDang97 mentioned this pull request Aug 7, 2026

c10k hardening — remaining Medium/Low findings (D3, D4, E1, E3, E4, F1–F6) #438

Open

qodo-code-review Bot mentioned this pull request Aug 7, 2026

fix(server): accept-loop HOL block, migration fd ownership, park auth gates (#438 F3–F6 + conn-secondary) #446

Merged

TinDang97 mentioned this pull request Aug 7, 2026

test(harness): pid-only --dir sweep + deadline-poll asserts (test-hygiene) #449

Merged

This was referenced Aug 15, 2026

fix(protocol): encode every reply in the protocol in effect when it was produced #496

Merged

fix(server): pipeline ordering at --shards >= 2 — inline commands ran ahead of the batch's pending remote writes (#507) #512

Merged

Navigation Menu

Uh oh!

fix(server): pipelined batch-tail hardening — remote-pending crash class, D4 migration gate, carried-remainder stall (#438) - #444

fix(server): pipelined batch-tail hardening — remote-pending crash class, D4 migration gate, carried-remainder stall (#438)#444
TinDang97 merged 2 commits into
mainfrom
fix/438-d4-migration-batch-tail

TinDang97 commented Aug 7, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Aug 7, 2026 •

edited

Loading

Review limit reached

Walkthrough

Changes

Sequence Diagram(s)

Uh oh!

qodo-code-review Bot commented Aug 7, 2026

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

qodo-code-review Bot commented Aug 7, 2026 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

TinDang97 commented Aug 7, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What the red test found

Red/green

Gates

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Aug 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review limit reached

Walkthrough

Changes

Sequence Diagram(s)

Uh oh!

qodo-code-review Bot commented Aug 7, 2026

PR Summary by Qodo

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

qodo-code-review Bot commented Aug 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Review by Qodo

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

TinDang97 commented Aug 7, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Aug 7, 2026 •

edited

Loading

qodo-code-review Bot commented Aug 7, 2026 •

edited

Loading