Fix sigterm-forwarding #54
Conversation
d-sparks
force-pushed
the
fix-sigterm-forwarding
branch
from
07a40bf
to
e8baaae
Compare
|
I updated the unit test to actually test the functionality, and also tested manually by building a local copy, connecting it to my local gearman, running jobs and killing the gearcmd PID and verifying that its child process's sigterm handling kicks in. |
|
I remember adding this back in the day (right after I joined Clever) and I dug up the original ticket that caused this change to be made: I think we will have to change gaprov-worker's bash script to forward the sigterm properly before we merge this change in. |
|
@johnhuangclever That's not necessary, gaprov-worker doesn't use |
| // be sent first folloed by SIGKILL after the grace period is over. If the grace period is 0, this | ||
| // is simply a hard SIGKILL. | ||
| func stopProcess(p *os.Process, gracePeriod time.Duration) error { | ||
| lg.InfoD("killing-pid", logger.M{"pgid": p.Pid}) |
There was a problem hiding this comment.
Don't call this pgid, instead call it pid.
|
LGTM after the log is fixed |
Previously, what was happening was that the SIGTERM was being forwarded from gearcmd to its entire process group, which sent a second, uncaught sigterm to gearcmd.
d-sparks
force-pushed
the
fix-sigterm-forwarding
branch
from
e8baaae
to
1d89bb3
Compare
Previously, what was happening was that the SIGTERM was being forwarded
from gearcmd to its entire process group, which sent a second, uncaught
sigterm to gearcmd.