Skip to content

Comments

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

Open
MitchLewis930 wants to merge 1 commit intopr_057_beforefrom
pr_057_after
Open

[Fix #3228] idle-timeout not working in cluster mode (#3235)#7
MitchLewis930 wants to merge 1 commit intopr_057_beforefrom
pr_057_after

Conversation

@MitchLewis930
Copy link

PR_057

@greptile-apps
Copy link

greptile-apps bot commented Jan 30, 2026

Greptile Overview

Greptile Summary

Fixed idle_timeout not working in cluster mode by implementing proper communication between worker and master processes when timeout is reached.

Key changes:

  • Added @idle_timeout_reached flag in lib/puma/server.rb:40 to track when idle timeout occurs
  • Modified lib/puma/server.rb:335 to set the flag when IO.select times out
  • Updated lib/puma/cluster/worker.rb:150 to send SIGTERM to master process when worker detects idle timeout
  • Added comprehensive tests for both cluster and single modes

How it works:
In cluster mode, each worker process runs its own Puma::Server instance. When a worker's server times out waiting for connections, it now sets the idle_timeout_reached flag. After the server thread completes, the worker checks this flag and sends SIGTERM to the master process if the timeout was reached, triggering a proper cluster-wide shutdown. Previously, only the individual worker would stop, leaving the master and other workers running.

Confidence Score: 5/5

  • This PR is safe to merge with minimal risk
  • The implementation is clean and well-tested. The fix properly addresses the issue by adding a simple flag to track idle timeout state and using it to signal the master process in cluster mode. The changes are minimal, focused, and include comprehensive tests for both single and cluster modes. The typo fix in lib/puma/runner.rb is a harmless documentation improvement.
  • No files require special attention

Important Files Changed

Filename Overview
lib/puma/server.rb Added idle_timeout_reached flag to track when idle timeout occurs, enabling cluster mode to properly shutdown master process
lib/puma/cluster/worker.rb Added SIGTERM signal to master process when worker detects idle timeout, implementing cluster shutdown

Sequence Diagram

sequenceDiagram
    participant Master as Master Process
    participant Worker as Worker Process
    participant Server as Puma::Server
    participant IOSelect as IO.select

    Note over Master,Server: Cluster Mode with idle_timeout configured

    Master->>Worker: Spawn worker process
    Worker->>Server: start_server()
    Server->>Server: @idle_timeout_reached = false
    
    Worker->>Server: server.run()
    Server->>IOSelect: IO.select(sockets, nil, nil, @idle_timeout)
    
    alt No new connections within timeout
        IOSelect-->>Server: nil (timeout reached)
        Server->>Server: @idle_timeout_reached = true
        Server->>Server: @status = :stop
        Server-->>Worker: server_thread.join completes
        
        Worker->>Worker: Check server.idle_timeout_reached
        alt idle_timeout_reached is true
            Worker->>Master: Process.kill("SIGTERM", master)
            Note over Master: Master receives SIGTERM and shuts down
        end
    else New connection received
        IOSelect-->>Server: [sockets] (connection ready)
        Server->>Server: Accept and process connection
        Note over Server: Continue running normally
    end
Loading

Copy link

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

2 files reviewed, no comments

Edit Code Review Agent Settings | Greptile

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.

2 participants