feat(command): real SHUTDOWN [NOSAVE|SAVE] + clean-shutdown AOF data-loss fix (task #27) by pilotspacex-byte · Pull Request #314 · pilotspace/moon

pilotspacex-byte · 2026-07-13T09:30:37Z

Task #27: SHUTDOWN implemented with Redis parity across all three dispatch paths (single/sharded/monoio): NOSAVE/SAVE/FORCE/NOW parsing, bare SHUTDOWN saves iff save points configured, failed forced save replies error and stays up, success reuses the SIGTERM CancellationToken graceful path with no reply (connection closes). ACL category DNG (already present); MULTI-queue fallback fails closed.

Bonus real bug fixed: all three AOF writer manifest-wait loops checked cancel.is_cancelled() BEFORE AofManifest::load, so a shutdown in the boot-time race window dropped already-queued Appends — silent data loss on a CLEAN shutdown (>80% repro). Fixed load-first in all three; stress-verified 25/25.

Verification: new tests/shutdown_integration.rs (4 real-server tests: NOSAVE waitpid exit, AOF durability across SHUTDOWN→restart, save-failure keeps server up, syntax errors keep server up) 5× green; test-consistency.sh SHUTDOWN section; 13 unit tests; fmt+clippy both matrices clean. Refs task #27.

Summary by CodeRabbit

New Features
- Added support for SHUTDOWN [NOSAVE|SAVE], including case-insensitive options and graceful connection closure.
- SAVE persists data before shutdown; failures return an error while keeping the server available.
- Added validation for conflicting or invalid shutdown options.
Bug Fixes
- Prevented queued append-only log writes from being lost during startup and shutdown races.
Tests
- Added integration coverage for shutdown behavior, persistence, save failures, and invalid arguments.

…ul stop Task #27: SHUTDOWN was a stub that always replied "ERR Errors trying to SHUTDOWN. Check logs." and never terminated the process. It is now intercepted at the connection-handler level (same pattern as BGSAVE/ACL) in all three dispatch paths -- handler_single, handler_sharded, handler_monoio: - Parses the optional NOSAVE/SAVE modifier (Redis parity: bare SHUTDOWN saves iff RDB save points are configured; NOSAVE always skips; SAVE always forces). ABORT is rejected (Moon's SHUTDOWN runs synchronously to completion -- there is never an in-progress shutdown to abort); FORCE/NOW are accepted no-ops. - A forced save that fails replies an error and the server stays up: single-shard uses the existing synchronous SAVE path; sharded/monoio use the cooperative per-shard BGSAVE snapshot, polled with a 10s bounded timeout (SHUTDOWN_SAVE_TIMEOUT_MS). - On success, triggers the exact same CancellationToken-driven graceful shutdown sequence already used for SIGTERM (per-shard WAL/AOF flush + fsync across every plane, accept-loop stop, clean connection teardown) instead of reimplementing it. No reply is sent on success -- Redis parity: the client observes the connection close. - ACL category was already admin/dangerous (DNG) in the command registry. Also fixes a real, pre-existing AOF-writer data-loss race surfaced while building the durability test: on first boot the AOF manifest is created on a separate thread from the one that starts accepting connections, and the writer's manifest-wait loop checked cancellation BEFORE attempting the load each iteration. A shutdown landing in that ~50ms window made the writer bail out without ever loading the by-then-current manifest, silently dropping every already-queued Append (reproduced >80% of runs: "AOF writer: cancelled while waiting for manifest" + a 0-byte incr file). Fixed in all three affected writer-loop variants (TopLevel-monoio, PerShard-tokio, PerShard-monoio) by trying the load first. Coverage: tests/shutdown_integration.rs (NOSAVE prompt exit + waitpid status, AOF durability across a SHUTDOWN->restart round trip -- stress tested 25x clean after the writer-task fix, a failed forced SAVE keeps the server up, syntax errors keep the server up) plus a cross-shard durability smoke section in scripts/test-consistency.sh. All four integration tests + 13 new/existing persistence unit tests green; cargo fmt, clippy (default + runtime-tokio feature matrices) clean. author: Tin Dang

qodo-code-review · 2026-07-13T09:30:43Z

Qodo reviews are paused for this user.

Troubleshooting steps vary by plan Learn more →

On a Teams plan?
Reviews resume once this user has a paid seat and their Git account is linked in Qodo.
Link Git account →

Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center?
These require an Enterprise plan - Contact us
Contact us →

coderabbitai · 2026-07-13T09:30:47Z

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 0abe0a72-b31a-4e49-8f5c-d7198e114752

📥 Commits

Reviewing files that changed from the base of the PR and between bb4a58c and 129395d.

📒 Files selected for processing (12)

CHANGELOG.md
scripts/test-commands.sh
scripts/test-consistency.sh
src/command/mod.rs
src/command/persistence.rs
src/persistence/aof/writer_task.rs
src/server/conn/handler_monoio/dispatch.rs
src/server/conn/handler_monoio/mod.rs
src/server/conn/handler_sharded/dispatch.rs
src/server/conn/handler_sharded/mod.rs
src/server/conn/handler_single.rs
tests/shutdown_integration.rs

📝 Walkthrough

Walkthrough

Changes

SHUTDOWN now supports NOSAVE and SAVE across single, sharded, and monoio handlers, performs required persistence, and initiates graceful termination without a client reply. AOF manifest polling was reordered to preserve queued appends during startup races, with integration and script coverage added.

SHUTDOWN and durability

Layer / File(s)	Summary
SHUTDOWN parsing and command contract `src/command/persistence.rs`, `src/command/mod.rs`	Adds save-mode parsing, default-save selection, polling constants, validation tests, and a fail-closed dispatch response.
Connection-level shutdown execution `src/server/conn/handler_single.rs`, `src/server/conn/handler_sharded/...`, `src/server/conn/handler_monoio/...`	Runs optional saves, reports failures without terminating the server, and cancels graceful shutdown on success.
AOF startup ordering and shutdown validation `src/persistence/aof/writer_task.rs`, `tests/shutdown_integration.rs`, `scripts/test-commands.sh`, `scripts/test-consistency.sh`, `CHANGELOG.md`	Loads AOF manifests before cancellation checks and adds shutdown exit, durability, failure, and syntax coverage.

Estimated code review effort: 4 (Complex) | ~45 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Client
  participant ConnectionHandler
  participant BGSAVE
  participant CancellationToken
  Client->>ConnectionHandler: SHUTDOWN SAVE or NOSAVE
  ConnectionHandler->>BGSAVE: Start and poll save when required
  BGSAVE-->>ConnectionHandler: Completion status
  ConnectionHandler->>CancellationToken: Cancel graceful shutdown
  ConnectionHandler-->>Client: Close connection without reply

✨ 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 feat/shutdown-command

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.}

…82a6c87be0b # Conflicts: # CHANGELOG.md

…82a6c87be0b

TinDang97 added 3 commits July 13, 2026 17:39

Merge remote-tracking branch 'origin/main' into worktree-agent-a11412…

0ad0738

…82a6c87be0b # Conflicts: # CHANGELOG.md

Merge remote-tracking branch 'origin/main' into worktree-agent-a11412…

3fa6947

…82a6c87be0b

Merge remote-tracking branch 'origin/main' into worktree-agent-a11412…

129395d

…82a6c87be0b

pilotspacex-byte merged commit 13ca6ec into main Jul 13, 2026
7 of 8 checks passed

TinDang97 deleted the feat/shutdown-command branch July 13, 2026 11:19

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

feat(command): real SHUTDOWN [NOSAVE|SAVE] + clean-shutdown AOF data-loss fix (task #27)#314

feat(command): real SHUTDOWN [NOSAVE|SAVE] + clean-shutdown AOF data-loss fix (task #27)#314
pilotspacex-byte merged 4 commits into
mainfrom
feat/shutdown-command

pilotspacex-byte commented Jul 13, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

qodo-code-review Bot commented Jul 13, 2026

Uh oh!

coderabbitai Bot commented Jul 13, 2026 •

edited

Loading

Review failed

Walkthrough

Changes

Sequence Diagram(s)

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

pilotspacex-byte commented Jul 13, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Uh oh!

qodo-code-review Bot commented Jul 13, 2026

Qodo reviews are paused for this user.

Uh oh!

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

Uh oh!

Review failed

Walkthrough

Changes

Sequence Diagram(s)

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

pilotspacex-byte commented Jul 13, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Jul 13, 2026 •

edited

Loading