search

python-diamond / Diamond

Diamond is a python daemon that collects system metrics and publishes them to Graphite (and others). It is capable of collecting cpu, memory, network, i/o, load and disk metrics. Additionally, it features an API for implementing custom collectors for gathering metrics from almost any source.
http://diamond.readthedocs.org/
MIT License
1.74k stars 601 forks source link

Fix handling of SIGINT on the main diamond process #633

Closed rksatyam closed 7 years ago

rksatyam commented 7 years ago

https://github.com/python-diamond/Diamond/commit/7db25796a9c20d81749a76eacf910b371e6652bb caused the regression where the main diamond process stopped handling SIGINT as intended.

The diff want to stop signal handling for the collector processes but was doing it in the collector constructor. The constructor is not the right place to reset as it is really constructed in the main process. Because of this, the signal handler of the main process was getting reset and the signal handler of the main process wasn't really getting called.

Deployed and tested that SIGINT on the main diamond process really kills all the child processes.

coveralls commented 7 years ago

Coverage Status

Coverage decreased (-0.1%) to 24.667% when pulling 09733d686823eee03e25489cb66682b0649d46b2 on rksatyam:fixv into 50e7eb5a44bb5d13491c4a62c25c64a0d3abc558 on python-diamond:master.

shortdudey123 commented 7 years ago

@benheilman any thoughts? This move of the signal handling makes sense

jaingaurav commented 7 years ago

@shortdudey123: Yeah reviewed this with @rksatyam the original patch seems to have been incorrect and this seems like the right way to achieve what the original patch was meant to do.

shortdudey123 commented 7 years ago

Sounds good, thats what it looked like

rksatyam commented 7 years ago

@shortdudey123, Thank you very much for the quick accept.

shortdudey123 commented 7 years ago

You are welcome :)