docs(issues): shutdown process — analysis, research, feature definition and sub-issue specs#1993
Draft
josecelano wants to merge 6 commits into
Draft
Conversation
Introduce two new top-level documentation directories: - docs/analysis/ — in-depth studies of the project's own code, typically produced before a refactoring plan or feature definition - docs/research/ — external research on technologies and patterns Each folder has an AGENTS.md explaining its purpose and the timestamp-prefix naming convention (same as ADRs). docs/AGENTS.md and docs/index.md are updated to register both folders. Ref: torrust#1488
Analyses the current shutdown implementation across src/main.rs, JobManager, all job starters, server launchers, and torrust_server_lib::signals. Key findings: - main.rs only handles SIGINT; SIGTERM is silently ignored - Three inconsistent shutdown mechanisms (CancellationToken, direct ctrl_c, oneshot Halted channel) - Grace period mismatch: JobManager 10s per-job vs Axum 90s - UDP server uses abort() with no connection draining - Double-signal race on Ctrl+C Includes experimental validation (2026-07-16, commit 49d8117): confirmed kill <pid> (SIGTERM) is ignored by main.rs while servers react independently via global_shutdown_signal(). Ref: torrust#1488
Investigates how long-running Rust services handle OS signals for graceful shutdown. Covers: - SIGINT vs SIGTERM vs SIGKILL semantics and differences - Tokio signal APIs (ctrl_c(), unix::signal(), CancellationToken) - Real-world patterns from Vector (Datadog): 4-signal handler, configurable 60s grace period, two-phase shutdown - Docker/K8s/systemd grace period reference values - Clarifies that kill is not brutal: it sends SIGTERM by default Recommendations for the Torrust Tracker: - Add SIGTERM handler to main.rs (minimal, highest value) - Remove global_shutdown_signal() from per-server shutdown - Make grace periods configurable Ref: torrust#1488
…sue specs Feature definition (docs/features/shutdown-process/): - README.md: product-level description of the shutdown feature including Unix/Docker/K8s/systemd contracts, principle of least surprise, user value table, production scenarios, shutdown trigger options analysis, and design ideas - open-questions.md: 10 open questions (3 critical, 4 important, 3 minor) that must be resolved before implementation begins; Q1 fully detailed with findings from running the standalone binary examples EPIC spec (docs/issues/open/1488-overhaul-tracker-shutdown/): - ISSUE.md: local spec for GitHub EPIC torrust#1488; updated goal, contracts being implemented, and sub-issue table with direct links to draft specs Sub-issue specs (docs/issues/open/ and docs/issues/drafts/): - 1588/ISSUE.md: local spec for GitHub issue torrust#1588 with manual verification procedure - SI-1 through SI-9: 9 draft sub-issue specs, each as a folder with ISSUE.md (spec + manual verification) and verification.md (evidence template); SI-1 includes Phase 1 pre-implementation evidence SI-1 pre-implementation evidence (2026-07-16, commit 49d8117): - Confirmed kill <pid> (SIGTERM) is ignored by main.rs - Servers react via global_shutdown_signal() but main.rs does not - Process requires kill -9 (exit code 137) to stop Ref: torrust#1488
There was a problem hiding this comment.
Pull request overview
This PR adds a structured set of planning artifacts (analysis, research, feature definition, and issue/sub-issue specs) for EPIC #1488 (Overhaul: Tracker Shutdown). It focuses on documenting the current shutdown shortcomings (notably SIGTERM handling) and defining a concrete implementation roadmap without changing production code.
Changes:
- Introduces
docs/analysis/anddocs/research/conventions and adds initial shutdown-focused analysis/research docs. - Adds a feature definition for “Shutdown Process”, including a tracked open-questions backlog.
- Adds/updates EPIC + sub-issue specification folders (SI-1 … SI-9) with verification templates.
Reviewed changes
Copilot reviewed 30 out of 30 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| project-words.txt | Adds new technical terms for spell-checking (needs re-sorting to maintain alphabetical order). |
| docs/AGENTS.md | Adds analysis/ and research/ to the docs navigation table. |
| docs/index.md | Adds new “Analysis Documents” and “Research Documents” sections. |
| docs/analysis/AGENTS.md | Defines purpose + naming/lifecycle conventions for analysis docs. |
| docs/analysis/20260716-shutdown-process/README.md | In-depth code-level analysis of current shutdown behavior and gaps. |
| docs/research/AGENTS.md | Defines purpose + naming/lifecycle conventions for research docs. |
| docs/research/20260716-console-shutdown-patterns/README.md | Research on SIGINT/SIGTERM semantics and common Rust/Tokio patterns. |
| docs/features/shutdown-process/README.md | Feature definition for shutdown behavior + design ideas. |
| docs/features/shutdown-process/open-questions.md | Open-questions tracker for unresolved design/contract decisions (Q1–Q10). |
| docs/issues/open/1488-overhaul-tracker-shutdown/ISSUE.md | Local EPIC spec tying analysis/research/feature docs to a sub-issue plan. |
| docs/issues/open/1588-review-shutdown-process-for-all-tasks-jobs/ISSUE.md | Local spec for sub-issue #1588 (inventory + migration planning). |
| docs/issues/open/1588-review-shutdown-process-for-all-tasks-jobs/verification.md | Verification evidence template for #1588. |
| docs/issues/drafts/1488-si-1-add-sigterm-to-main/ISSUE.md | Draft SI-1 spec: add SIGTERM handling to main.rs. |
| docs/issues/drafts/1488-si-1-add-sigterm-to-main/verification.md | SI-1 verification template incl. pre-implementation evidence. |
| docs/issues/drafts/1488-si-2-remove-global-shutdown-signal/ISSUE.md | Draft SI-2 spec: remove per-server global signal shutdown path. |
| docs/issues/drafts/1488-si-2-remove-global-shutdown-signal/verification.md | Verification template for SI-2. |
| docs/issues/drafts/1488-si-3-fix-environment-stop/ISSUE.md | Draft SI-3 spec: fix standalone examples’ shutdown behavior. |
| docs/issues/drafts/1488-si-3-fix-environment-stop/verification.md | Verification template for SI-3. |
| docs/issues/drafts/1488-si-4-migrate-torrent-cleanup/ISSUE.md | Draft SI-4 spec: migrate torrent cleanup job to CancellationToken. |
| docs/issues/drafts/1488-si-4-migrate-torrent-cleanup/verification.md | Verification template for SI-4. |
| docs/issues/drafts/1488-si-5-migrate-activity-metrics-updater/ISSUE.md | Draft SI-5 spec: migrate activity metrics updater to CancellationToken. |
| docs/issues/drafts/1488-si-5-migrate-activity-metrics-updater/verification.md | Verification template for SI-5. |
| docs/issues/drafts/1488-si-6-align-grace-periods/ISSUE.md | Draft SI-6 spec: align JobManager grace periods with Axum drain timeouts. |
| docs/issues/drafts/1488-si-6-align-grace-periods/verification.md | Verification template for SI-6. |
| docs/issues/drafts/1488-si-7-observable-shutdown-progress/ISSUE.md | Draft SI-7 spec: add observable shutdown progress logging. |
| docs/issues/drafts/1488-si-7-observable-shutdown-progress/verification.md | Verification template for SI-7. |
| docs/issues/drafts/1488-si-8-configurable-grace-periods/ISSUE.md | Draft SI-8 spec: add [shutdown] config section for grace periods. |
| docs/issues/drafts/1488-si-8-configurable-grace-periods/verification.md | Verification template for SI-8. |
| docs/issues/drafts/1488-si-9-improve-udp-shutdown/ISSUE.md | Draft SI-9 spec: improve UDP shutdown observability/draining decisions. |
| docs/issues/drafts/1488-si-9-improve-udp-shutdown/verification.md | Verification template for SI-9. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
361
to
369
| shellcheck | ||
| SHLVL | ||
| Signedness | ||
| pgrep | ||
| rwxrwxr | ||
| SIGUSR | ||
| skiplist | ||
| taskkill | ||
| slowloris |
| | [Q7](#q7) | 🟡 Important | Open | `#[cfg(unix)]` asymmetry on Windows not noted | | ||
| | [Q8](#q8) | 🟢 Minor | Open | `SIGHUP` / config reload not explicitly deferred | | ||
| | [Q9](#q9) | 🟢 Minor | Open | Docker experimental validation missing | | ||
| | [Q10](#q10) | 🟢 Minor | Open | Option 4 heading/body mismatch after signal rename | |
Comment on lines
+515
to
+532
| ## Q10 | ||
|
|
||
| **Severity**: 🟢 Minor | ||
| **Status**: Open | ||
| **Title**: Option 4 heading/body mismatch after signal rename | ||
|
|
||
| ### Description | ||
|
|
||
| In the feature doc "Shutdown Triggers" section, Option 4 was renamed from | ||
| "Custom Signals (SIGUSR1 / SIGUSR2)" to "Custom Signals" to avoid cspell issues. | ||
| However, the body still refers to `SIGUSR1` and `SIGUSR2` by name. The heading | ||
| no longer tells the reader which signals are being discussed. | ||
|
|
||
| ### Action | ||
|
|
||
| - [ ] Restore the signal names to the heading: "Option 4: Custom Signals | ||
| (`SIGUSR1` / `SIGUSR2`)" — the cspell issue is solved by the `SIGUSR` entry | ||
| in `project-words.txt`, so the full name can now be used in the heading. |
Comment on lines
+372
to
+376
| ```toml | ||
| [shutdown] | ||
| grace_period_secs = 30 | ||
| force_timeout_secs = 35 | ||
| ``` |
…l() removal are sequential Decision recorded in open-questions.md Q1: - SI-1 (add SIGTERM to main.rs) can land before SI-2 without breaking correctness. The triple-signal scenario after SI-1 is harmless: main.rs reacts first, servers' global_shutdown_signal() fires as a no-op. - SI-1 and SI-2 are separate sequential sub-issues. SI-2 requires a coordinated torrust-server-lib release and Q5 resolution. - SI-3 is fully unblocked: Environment::stop() abort-vs-cancel and SIGTERM for standalone examples can land independently of SI-2. Updated: - open-questions.md: Q1 marked resolved, decisions recorded, Question to Sub-issue mapping table and Sub-issue Readiness table added - EPIC ISSUE.md: sub-issue table Notes updated (SI-2 blocked by Q5 only, SI-3 unblocked) - SI-2 ISSUE.md: blocked notice updated (Q1 resolved, Q5 remains) - SI-3 ISSUE.md: unblocked notice added, dependencies updated Ref: torrust#1488
…server job Decision recorded in open-questions.md Q2 (Option 3): Each server job starter watches the CancellationToken from JobManager. When cancelled, it sends Halted::Normal to its own halt channel and waits for the server task to finish. JobManager API (JoinHandle<()>) is unchanged. No changes needed to torrust_server_lib or torrust_tracker_axum_server. Options 1 and 2 rejected: - Option 1: halt senders are consumed inside async move closures; extracting them would require breaking API changes to all start_job functions - Option 2: requires touching two external standalone packages unnecessarily Impact on SI-2: must now also add CancellationToken wiring to each server job starter alongside the global_shutdown_signal() removal in torrust-server-lib. Updated: - open-questions.md: Q2 marked resolved, decision and rationale recorded, impact table updated - SI-2 ISSUE.md: goal, implementation, and acceptance criteria updated to include CancellationToken wiring in job starters Ref: torrust#1488
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR consolidates the planning and specification work for EPIC #1488
(Overhaul: Tracker Shutdown). It does not change any production code.
Motivation
The
kill <pid>command — the most basic Unix way to ask a process to stop —silently does nothing to the Torrust Tracker today.
main.rsonly handlesSIGINT(Ctrl+C) and ignoresSIGTERM. This breaks the Unix, Docker,Kubernetes, and systemd process lifecycle contracts.
This PR captures the analysis, research, feature definition, and concrete
sub-issue specs needed before implementation begins.
What is included
New documentation structure
docs/analysis/— for in-depth studies of the project's own codedocs/research/— for external research on technologies and patterns(both use timestamp-prefix naming, like ADRs)
Analysis (docs/analysis/20260716-shutdown-process/)
Code-level analysis of the current shutdown process:
main.rsbut caught independently by each serverResearch (docs/research/20260716-console-shutdown-patterns/)
External research on signal handling patterns:
Feature definition (docs/features/shutdown-process/)
README.md: product-level description including Unix/Docker/K8s/systemdcontracts, principle of least surprise, and design ideas
open-questions.md: 10 open questions (3 critical, 4 important, 3 minor)that must be resolved before implementation begins
Issue specs (docs/issues/)
ISSUE.md: spec, implementation guidance, acceptance criteria, manualverification procedure
verification.md: evidence template (SI-1 includes pre-implementationevidence from the 2026-07-16 experiment)
Relation to #1488
This PR directly prepares the implementation of EPIC #1488. The sub-issue
ready to implement immediately with no blockers is SI-1 (add SIGTERM
handler to
main.rs) — a small, high-value change that fixes the Unixcontract.
Status
Draft — the open-questions.md tracks 10 items still to be resolved before
implementation begins. Reviewers are invited to comment on the analysis,
research conclusions, and sub-issue specs.