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

[joshuay03]

Description

Closes #3228.

Ensures that a worker kills the master process if its server shut down due the the idle timeout elapsing. The master will gracefully shut down the rest of the workers and then itself.

I wonder if we should backport this to https://github.com/puma/puma/tree/v6.4.0 🤔

Stupid question 🤦🏽‍♂️

Your checklist for this pull request

• I have reviewed the guidelines for contributing to this repository.
• I have added (or updated) appropriate tests if this PR fixes a bug or adds a feature.
• My pull request is 100 lines added/removed or less so that it can be easily reviewed.
• If this PR doesn't need tests (docs change), I added [ci skip] to the title of the PR.
• If this closes any issues, I have added "Closes #issue" to the PR description or my commit messages.
• I have updated the documentation accordingly.
• All new and existing tests passed, including Rubocop.

[joshuay03]

Trapped here:

puma/lib/puma/cluster.rb

Lines 323 to 341 in 252890c

	master_pid = Process.pid
	Signal.trap "SIGTERM" do
	# The worker installs their own SIGTERM when booted.
	# Until then, this is run by the worker and the worker
	# should just exit if they get it.
	if Process.pid != master_pid
	log "Early termination of worker"
	exit! 0
	else
	@launcher.close_binder_listeners
	stop_workers
	stop
	@events.fire_on_stopped!
	raise(SignalException, "SIGTERM") if @options[:raise_exception_on_sigterm]
	exit 0 # Clean exit, workers were stopped
	end
	end

[joshuay03]

Before:

[nateberkopec]

I wonder if we should backport this to https://github.com/puma/puma/tree/v6.4.0 🤔

Sorry, what do you mean?

[joshuay03]

I wonder if we should backport this to v6.4.0 🤔

Sorry, what do you mean?

Sorry, I might be misunderstanding how releases work...

What I'm trying to say is, since the feature was included in https://github.com/puma/puma/releases/tag/v6.4.0, is there a way to include this patch in the same release? 🤔

[dentarg]

We (Nate) will have to put out a patch release, 6.4.1.

[davidalejandroaguilar]

Thanks for the great and quick work on this! 🥇

[nateberkopec]

See #3261

[bensheldon]

Sorry for the question here. Am I understanding that if any single worker hits the idle timeout, it shuts down everything?

I was trying this locally (bundle exec puma -C config/puma.rb -w 10 --idle-timeout 10) and it seems like if the server gets 1 request every 5 seconds (less than the 10 second idle timeout), it will still shutdown. That doesn't seem right.

It seems like the the Puma::Cluster should be checking across all of the workers and then terminating if idle.

[dentarg]

That sounds like a bug to me indeed, and worthy of its own issue so it can be picked up! (Disclaimer: I haven't tested anything myself in regards to this feature)

[bensheldon]

I opened #3282

[joshuay03]

Am I understanding that if any single worker hits the idle timeout, it shuts down everything?

Yes that is how this currently works. But,

It seems like the the Puma::Cluster should be checking across all of the workers and then terminating if idle.

this does make much more sense to me.

I may have jumped the gun on this one before clarifying the exact implementation based off of #3228 (comment) and #3228 (comment).

I'll tackle #3282 sometime today/tomorrow if no one's picked it up by then.