Page MenuHomePhabricator

Drive "phd stop" entirely from the process list, not PID files on disk

Authored by epriestley on Jun 20 2019, 9:14 PM.



Ref T13321. Depends on D20600. Make bin/phd stop mean:

  • bin/phd stop: Stop all processes which have daemon process titles. If we're instanced, only stop daemons for the current instance.
  • bin/phd stop --force: Stop all processes which have deamon process titles for any instance.

We no longer read or care about PID files on disk, and this moves us away from PID files.

This makes unusual flag --gently do nothing. A followup will update the documentation and flags to reflect actual usage/behavior.

This also removes the ability to stop specific PIDs. This was somewhat useful long, long ago when you might explicitly run different copies of the PullLocal daemon with flags to control which repositories they updated, but with the advent of clustering it's no longer valid to run custom daemon loadouts.

Test Plan

Ran bin/phd start, then bin/phd stop. Saw instance daemons stop. Ran bin/phd stop --force, saw all daemons stop.

Diff Detail

rP Phabricator
Lint Not Applicable
Tests Not Applicable

Event Timeline

amckinley added inline comments.

Don't you think we should distinguish between "no daemons found" and "we couldn't run the query"? If for no other reason than to more-quickly shake out weird distros where our ps invocation doesn't work. (Incidentally, it does work on my Mac, so hooray for that).


"for all instances"

This revision is now accepted and ready to land.Jun 20 2019, 10:22 PM

We'll still phlog($ex), so Solaris should get something like this:

EXCEPTION: ps doesn't take these flags on solaris ha ha ha

...which should be reasonably clear, and if anyone hits this maybe we can write phutil_is_solaris() and find the right ps incantation on Solaris.

On Windows I think we just fatal instantly anyway since there's no posix or pcntl.

So, practically, I don't think any paths today will lead to messaging that just says "no stuff" when the root cause is really "ProcessQuery can't deal with it", although this is sort of a product of me arranging things to hit a convenient series of coincidences rather than an explicit rule of the system.

  • Wordsmithing: "any instance" -> "all instances"