Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add --idle-timeout CLI option to create temporary processes that exit when inactive #1159

Merged
merged 15 commits into from
Dec 2, 2023

Conversation

bensheldon
Copy link
Owner

@bensheldon bensheldon commented Nov 26, 2023

Replaces #1146. Connects to #1140. Naming inspired by puma/puma#3209

The idea here is that, especially in a containerized environment, temporary instances of the GoodJob CLI can be spun up to execute jobs, and then they will automatically exit when no jobs remain to be executed.

Note that this feature is ignorant of the polling interval or future-scheduled jobs. So if the exit-on-idle duration is less than the polling interval and LISTEN/NOTIFY is disabled, it's possible for the process to exit even though jobs exist on the queue.

A very naive way to monitor for idle processes
Expose stats, and tweak the logic for checking idle
Put the formatting back so that it matches the rest of the codebase
While trying to get the test running, I found that the reference to the
Gemfile was going one directory to high.  I've corrected it here and the
bin/setup command works now.
Instead of pushing the logic of idle all the way up to the CLI I thought
it would be better in the GoodJob::Capsule class.

I've created a `#idle?` method that does this check based on the last
time a scheduler in the GoodJob::Capsule has run a job.

I ran into an interesting thing that if I check the configuration object
in the initialization of the GoodJob::Capsule it causes a crash because
rails is not initialized yet.  The configuation object can't access the
rails config.

I instead changed it so that in the initialize of the GoodJob::Capsule
the `shutdown_on_idle_enabled` flag is set to false - meaning never
shutdown on idle. Then in `#start` the GoodJob::Capsule checks the
config and sets the flag so that #idle? method will work.
I had a stale file open and overwrote the check of the event.  This was
after I had pushed up all the code.

This also made me realize that, now one can't set less than a 10 second
idle time out.  The CLI will always stay up for at least 10 seconds.  As
mentioned in a previous issue, this may cause a support request when
someone sets a idle time out of 1 second but the CLI stays up for 10
seconds then shuts down.

In real life I can't imagine that someone would want to have a job
server around for less than 10 seconds. Maybe this just needs to be
surfaced in the documentation for the command line option.

In any case, I've added code so that the wait will take the
shutdown_on_idle as the time out to check the event, or it will take the
default of 10 seconds.

Also in my testing I realized that the `executed_at` was being set in
every loop because of the `increment_empty_executions`, this meant that
the time out would never happen during actual execution.

I've removed the setting of the `executed_at` in this function as
technically nothing executed.
Over wrote something by mistake
@bensheldon
Copy link
Owner Author

Going to rename this from --exit-on-idle to --idle-timeout to match the name of Puma's option.

@bensheldon bensheldon changed the title Add --exit-on-idle CLI option for creating temporary processes Add --idle-timeout CLI option to create temporary processes that exit when inactive Nov 26, 2023
@bensheldon bensheldon marked this pull request as ready for review November 27, 2023 01:43
@bensheldon bensheldon merged commit 777ddfe into main Dec 2, 2023
20 checks passed
@bensheldon bensheldon deleted the Paradem/main branch December 2, 2023 16:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
Development

Successfully merging this pull request may close these issues.

2 participants