thuehlinger / daemons

Ruby daemons gem official repository
MIT License
648 stars 71 forks source link

Failure to find pid files when status or stop is specified without a number argument #40

Closed cthielen closed 5 years ago

cthielen commented 9 years ago

If I start a daemon with '-n 5', I get 5 processes and 5 pid files.

However, if I then simply type "status", no processes are found. Consequently, "stop" fails to work as well.

If I run "stop -n 5", (or -n 100 for that matter), daemons does correctly find the processes.

Is this the intended behavior? Presumably 'stop' and 'status' should both work without a number being specified.

philister commented 8 years ago

We ran into that, when "script/delayed_job stop" and "script/delayed_job status" stopped working.

srswanson commented 8 years ago

+1 on delayed_job stop and delayed_job status no longer working with version 1.2.3. Delayed_job works properly after reverting back to daemons v1.1.9.

joshuasiler commented 8 years ago

Looks like this is the commit that broke delayed_jobs https://github.com/thuehlinger/daemons/commit/55382e5a8a6baa8c01612f3d39578c967685cfcb

Delayed jobs creates pid files that are delayed_job.1.pid, delayed_job.2.pid etc.

thuehlinger commented 8 years ago

I feel that this should be fixed in delayed_jobs, not in daemons. Before 1.2.3, the .pid file matching in daemons was too loose (glob for process_name*.pid). This has been corrected in 1.2.3. If delay_jobs is using process names with ascending numbers for start, it should also use them for status and stop.

tardate commented 5 years ago

yes, also just run into this via delayed_job. Looking at the respective changes, it appears as if the issue is just a pid file naming convention clash?

DelayedJob still uses: delayed_job.1.pid, delayed_job.2.pid etc. This is now a miss-match on the glob in daemons 1.2.3+ https://github.com/thuehlinger/daemons/blob/master/lib/daemons/pidfile.rb#L34 as daemons now expects (the very ugly IMHO!) delayed_job_num1.pid, delayed_job_num2.pid etc.

So if I'm reading this right, if DelayedJob switched its forced filename convention to match the form now expected by daemons ... it should all just work. @thuehlinger does that make sense? I'll try to test that theory now..

tardate commented 5 years ago

@thuehlinger ran a little test - see https://github.com/evendis/delayed_job/commit/87a396a2682063e12749847e576d163fce5e8f64 which seems to confirm this is just a pid-file naming convention disagreement.

The commit switches delayed job to use the new convention for daemons 1.2.3+ (<base>_num<i>), and solves the start/stop issue.

However, changing the process/pid file naming convention for DelayedJob is probably not a very good idea. I know it is very common for scripts to be looking for delayed_job processes, and since it is a very old gem in wide use, there is an unknown amount of churn that would be created by changing the process name format.

Perhaps a better approach would be to expose the globing format as a configurable option in daemons. It could default to the new <base>_num<i>, but downstream projects like DelayedJob could poke in their own format (without resorting to monkey patches) and thus preserve their own legacy compatibility. What do you think? I can offer a PR if you think this approach has merit.

thuehlinger commented 5 years ago

@tardate Thanks for the testing and foremost for the pull request. I think your analysis and fix is reasonable. I have just merged it and I am about to prepare a release 1.3.0 which also includes your other merge request.

ultrawebmarketing commented 5 years ago

I'm using daemons 1.3.1 and delayed_job 4.1.5 and still having this issue. Is there something i'm missing?

dnt294 commented 11 months ago

daemons 1.4.1 and delayed_job 4.1.10 and still having this issue. 👀