For being able to notify (/waking) a sleeping daemon, a signal handler for the SIGUSR2 is added to the DaemonOverseer. This signal will be send to the child process which can implement a signal handler.
Details
Details
- Reviewers
epriestley - Group Reviewers
Blessed Reviewers - Commits
- rPHU5085f26935d4: Add a signal handler for SIGUSR2.
Put a PhutilDaemon (with a signal handler on SIGUSR2) to sleep for 10 minutes. Meanwhile, send a SIGURS2 to the daemon and see it being waked up.
Diff Detail
Diff Detail
- Lint
Lint Skipped - Unit
Tests Skipped
Event Timeline
Comment Actions
What's the overall goal here? Are you developing new daemons? This code looks fine, but I want to make sure the followups bring us in a sensible direction.
Particularly, shoving things into the task queue should already accomplish this, effectively. Are there reasons you can't do this work via the task queue?
Comment Actions
Actually, I'm only using the libphutil in our project, in which I am developing daemons. The task queue you're talking about is included in Phabricators library isn't it? (Correct me if I'm wrong, I'm just using the PhutilDaemon and PhutilDaemonOverseer). I've buid a
The overall goal is to not have the daemons wake up and poll the application's database every second as there will be periods of hours when there won't be any pending tasks. So in our project we would like to have the option to manually notify the workers when there is a new batch of tasks available.
Comment Actions
Okay, I think this is reasonable. I think the only two plausible behaviors for SIGUSR2 are "pass it to the daemons" or "receive it from the daemons", and I can't imagine any other message we'd want to receive from the daemons in the overseer (other than the "not hung" message indicated by SIGUSR1). There are various other signals we could use safely (HUP, URG, WINCH) later if we do need more stuff, and it wouldn't be terribly difficulty to switch which signals we use if necessary.
However, I think we should install the listener for this signal unconditionally in PhutilDaemon, and have it call some didReceiveNotifySignal() method with a default empty implementation. This will let us change the mechanism later without impacting your code, and plausibly cover this with unit tests -- basically, make the mechanism agnostic to the actual transport mechanic.
Broadly, I hesitate a bit to accept changes in the upstream which aren't used by Phabricator, since we're more likely to break them by accident, but I think this one is reasonable, especially if we can obscure the mechanism from the daemons.
Comment Actions
Make the notify mechanism for daemons transport agnostic
By implementing the signal handler for the SIGUSR2 signal in the PhutilDaemon, subclasses only need to hook into the onNotify function.
Also, because of the hearbeat signal every minute while sleeping, we need to keep track whether the notify signal is received while sleeping. Else, a sleep longer than 60 seconds will only be shortened by 60 seconds on a notify signal instead of continuing the run method of the implemented daemon.
Comment Actions
Closed by commit rPHU5085f26935d4 (authored by @krisbuist, committed by @epriestley).