[Fix #3228] idle-timeout not working in cluster mode (#3235)

[MitchLewis930]

PR_057

Summary by CodeRabbit

• New Features

  • Server now performs a graceful shutdown when idle timeout is reached.
  • Idle timeout status is exposed for monitoring.

• Tests

  • Added integration tests verifying idle-timeout behavior in single and clustered modes.

[coderabbitai]

📝 Walkthrough

Walkthrough

Adds idle-timeout detection and shutdown orchestration: Server exposes idle_timeout_reached and marks status to stop when idle; handle_servers schedules graceful shutdown. Worker ensures the master receives SIGTERM when idle timeout is reached. Also fixes a comment typo and adds integration tests for idle timeout behavior.

Changes

Cohort / File(s)	Summary
Server & Worker runtime changes lib/puma/server.rb, lib/puma/cluster/worker.rb	Adds attr_reader :idle_timeout_reached and sets it on accept-loop idle timeout; transitions server status to :stop and schedules graceful shutdown. Worker sends SIGTERM to master in the worker shutdown ensure path when idle timeout reached.
Runner comment fix lib/puma/runner.rb	Typo corrected in an explanatory comment: "aand" → "and". No behavioral change.
Integration tests test/test_integration_cluster.rb, test/test_integration_single.rb	Adds test_idle_timeout to verify the server refuses connections after the configured idle timeout in both cluster and single modes.

Sequence Diagram

sequenceDiagram
    actor Client
    participant Server
    participant Worker
    participant Master

    Client->>Server: establish connection
    Note over Server: accept loop waiting for activity

    rect rgba(255,165,0,0.5)
        Note over Server: idle_timeout expires
        Server->>Server: set idle_timeout_reached = true
        Server->>Server: status = :stop
    end

    rect rgba(100,150,255,0.5)
        Server->>Server: handle_servers -> graceful_shutdown
        Server->>Server: stop accepting connections
    end

    rect rgba(150,100,255,0.5)
        Worker->>Master: Process.kill("SIGTERM", master)
        Master->>Master: initiate shutdown
    end

    Client->>Server: reconnect attempt
    Server-->>Client: ECONNREFUSED

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Poem

🐰 Idle fields hush, no requests in sight,
I tap the master with a gentle SIGTERM flight,
A graceful stop, then curtains descend,
Worker and server in tidy end,
Hoppity code—sleep well through the night.

🚥 Pre-merge checks | ✅ 2 | ❌ 2

❌ Failed checks (2 warnings)

Check name	Status	Explanation	Resolution
Description check	⚠️ Warning	The pull request description is minimal and incomplete, containing only 'PR_057' without addressing required template sections or providing context about the changes.	Complete the description following the template: explain the original problem, link related issues, describe alternatives considered, and justify design choices. Add checklist confirmations for tests, documentation, and contribution guidelines.
Docstring Coverage	⚠️ Warning	Docstring coverage is 9.09% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and specifically describes the main change: fixing the idle-timeout feature in cluster mode, with reference to issue #3228.
Merge Conflict Detection	✅ Passed	✅ No merge conflicts detected when merging into pr_057_before

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch pr_057_after

---

No actionable comments were generated in the recent review. 🎉

📜 Recent review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 87ad26a and 8d1f77b.

📒 Files selected for processing (5)

• lib/puma/cluster/worker.rb
• lib/puma/runner.rb
• lib/puma/server.rb
• test/test_integration_cluster.rb
• test/test_integration_single.rb

🧰 Additional context used

🧬 Code graph analysis (3)

test/test_integration_single.rb (2)

test/test_integration_cluster.rb (1)

• test_idle_timeout (224-236)

test/helpers/integration.rb (2)

• cli_server (73-114)
• connect (200-205)

test/test_integration_cluster.rb (3)

test/helpers/integration.rb (1)

• cli_server (73-114)

test/test_integration_pumactl.rb (1)

• workers (8-8)

lib/puma/dsl.rb (1)

• workers (584-586)

lib/puma/cluster/worker.rb (1)

lib/puma/cluster/worker_handle.rb (1)

• kill (86-89)

🔇 Additional comments (8)

lib/puma/runner.rb (1)

73-73: LGTM!

Good catch on the typo fix: "aand" → "and".

lib/puma/server.rb (4)

40-40: LGTM!

The new attr_reader :idle_timeout_reached properly exposes the idle timeout state for external consumers (e.g., the worker process).

---

121-122: LGTM!

Proper initialization of @idle_timeout_reached to false in the constructor.

---

334-339: LGTM!

The idle timeout handling logic is correct:

• Only triggers when IO.select returns nil (timeout with no activity)
• Guards with unless shutting_down? to avoid overwriting state during an already-in-progress shutdown
• Sets the flag and transitions to :stop status for graceful shutdown

---

377-378: LGTM!

The graceful_shutdown call already handled :stop status, so no behavioral change here—just a formatting adjustment.

lib/puma/cluster/worker.rb (1)

149-150: LGTM!

This is the core fix for idle timeout in cluster mode. When a worker's server reaches idle timeout, it signals the master process with SIGTERM to initiate a cluster-wide graceful shutdown. The placement after the before_worker_shutdown hooks ensures shutdown hooks complete before signaling the master.

test/test_integration_single.rb (1)

257-267: LGTM!

The test correctly verifies idle timeout behavior in single mode:

• Configures 1-second idle timeout
• Establishes a connection, then waits slightly longer than the timeout
• Asserts that new connections are refused after idle timeout triggers shutdown

The test follows the same pattern as the existing test_pre_existing_unix_after_idle_timeout in this file.

test/test_integration_cluster.rb (1)

224-236: LGTM!

The test correctly verifies idle timeout behavior in cluster mode:

• Launches server with 2 workers (-w #{workers})
• Waits for workers to boot via get_worker_pids
• Establishes connection, waits beyond timeout, then asserts connection refused

This test exercises the new Process.kill "SIGTERM", master code path in lib/puma/cluster/worker.rb that fixes the idle timeout behavior in cluster mode.

_{✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.}

---

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

[MitchLewis930]

@coderabbitai full review

[coderabbitai]

✅ Actions performed

Full review triggered.

[MitchLewis930]

@coderabbitai full review

[coderabbitai]

✅ Actions performed

Full review triggered.