Fix duplicate kills of collection threads on shutdown (#6387)
Disables collection threads killed with SIGTERM to stop the cleanup function from attempting to kill them again.
emmrk committed
Jul 9, 2019 at 12:03 UTC
34642e336532a711cfa163d1ce055f006a6f83a8
1 file changed
+69
-41
collectors/plugins.d/plugins_d.c
+69
-41
@@ -526,6 +526,70 @@ static void pluginsd_worker_thread_cleanup(void *arg) {
526
}
527
}
528
529
+#define SERIAL_FAILURES_THRESHOLD 10
530
+static void pluginsd_worker_thread_handle_success(struct plugind *cd) {
531
+ if (likely(cd->successful_collections)) {
532
+ sleep((unsigned int) cd->update_every);
533
+ return;
534
+ }
535
+
536
+ if(likely(cd->serial_failures <= SERIAL_FAILURES_THRESHOLD)) {
537
+ info("'%s' (pid %d) does not generate useful output but it reports success (exits with 0). %s.",
538
+ cd->fullfilename, cd->pid,
539
+ cd->enabled ?
540
+ "Waiting a bit before starting it again." :
541
+ "Will not start it again - it is now disabled.");
542
+ sleep((unsigned int) (cd->update_every * 10));
543
+ return;
544
+ }
545
+
546
+ if (cd->serial_failures > SERIAL_FAILURES_THRESHOLD) {
547
+ error("'%s' (pid %d) does not generate useful output, although it reports success (exits with 0)."
548
+ "We have tried to collect something %zu times - unsuccessfully. Disabling it.",
549
+ cd->fullfilename, cd->pid, cd->serial_failures);
550
+ cd->enabled = 0;
551
+ return;
552
+ }
553
+
554
+ return;
555
+}
556
+
557
+static void pluginsd_worker_thread_handle_error(struct plugind *cd, int worker_ret_code) {
558
+ if (worker_ret_code == -1) {
559
+ info("'%s' (pid %d) was killed with SIGTERM. Disabling it.", cd->fullfilename, cd->pid);
560
+ cd->enabled = 0;
561
+ return;
562
+ }
563
+
564
+ if (!cd->successful_collections) {
565
+ error("'%s' (pid %d) exited with error code %d and haven't collected any data. Disabling it.",
566
+ cd->fullfilename, cd->pid, worker_ret_code);
567
+ cd->enabled = 0;
568
+ return;
569
+ }
570
+
571
+ if (cd->serial_failures <= SERIAL_FAILURES_THRESHOLD) {
572
+ error("'%s' (pid %d) exited with error code %d, but has given useful output in the past (%zu times). %s",
573
+ cd->fullfilename, cd->pid, worker_ret_code, cd->successful_collections,
574
+ cd->enabled ?
575
+ "Waiting a bit before starting it again." :
576
+ "Will not start it again - it is disabled.");
577
+ sleep((unsigned int) (cd->update_every * 10));
578
+ return;
579
+ }
580
+
581
+ if (cd->serial_failures > SERIAL_FAILURES_THRESHOLD) {
582
+ error("'%s' (pid %d) exited with error code %d, but has given useful output in the past (%zu times)."
583
+ "We tried to restart it %zu times, but it failed to generate data. Disabling it.",
584
+ cd->fullfilename, cd->pid, worker_ret_code, cd->successful_collections, cd->serial_failures);
585
+ cd->enabled = 0;
586
+ return;
587
+ }
588
+
589
+ return;
590
+}
591
+#undef SERIAL_FAILURES_THRESHOLD
592
+
593
void *pluginsd_worker_thread(void *arg) {
594
netdata_thread_cleanup_push(pluginsd_worker_thread_cleanup, arg);
595
@@ -546,50 +610,14 @@ void *pluginsd_worker_thread(void *arg) {
610
error("'%s' (pid %d) disconnected after %zu successful data collections (ENDs).", cd->fullfilename, cd->pid, count);
611
killpid(cd->pid, SIGTERM);
612
549
- // get the return code
550
- int code = mypclose(fp, cd->pid);
613
+ int worker_ret_code = mypclose(fp, cd->pid);
614
552
- if(code != 0) {
553
- // the plugin reports failure
554
-
555
- if(likely(!cd->successful_collections)) {
556
- // nothing collected - disable it
557
- error("'%s' (pid %d) exited with error code %d. Disabling it.", cd->fullfilename, cd->pid, code);
558
- cd->enabled = 0;
559
- }
560
- else {
561
- // we have collected something
615
+ if (likely(worker_ret_code == 0))
616
+ pluginsd_worker_thread_handle_success(cd);
617
+ else
618
+ pluginsd_worker_thread_handle_error(cd, worker_ret_code);
619
563
- if(likely(cd->serial_failures <= 10)) {
564
- error("'%s' (pid %d) exited with error code %d, but has given useful output in the past (%zu times). %s", cd->fullfilename, cd->pid, code, cd->successful_collections, cd->enabled?"Waiting a bit before starting it again.":"Will not start it again - it is disabled.");
565
- sleep((unsigned int) (cd->update_every * 10));
566
- }
567
- else {
568
- error("'%s' (pid %d) exited with error code %d, but has given useful output in the past (%zu times). We tried %zu times to restart it, but it failed to generate data. Disabling it.", cd->fullfilename, cd->pid, code, cd->successful_collections, cd->serial_failures);
569
- cd->enabled = 0;
570
- }
571
- }
572
- }
573
- else {
574
- // the plugin reports success
575
-
576
- if(unlikely(!cd->successful_collections)) {
577
- // we have collected nothing so far
578
-
579
- if(likely(cd->serial_failures <= 10)) {
580
- error("'%s' (pid %d) does not generate useful output but it reports success (exits with 0). %s.", cd->fullfilename, cd->pid, cd->enabled?"Waiting a bit before starting it again.":"Will not start it again - it is now disabled.");
581
- sleep((unsigned int) (cd->update_every * 10));
582
- }
583
- else {
584
- error("'%s' (pid %d) does not generate useful output, although it reports success (exits with 0), but we have tried %zu times to collect something. Disabling it.", cd->fullfilename, cd->pid, cd->serial_failures);
585
- cd->enabled = 0;
586
- }
587
- }
588
- else
589
- sleep((unsigned int) cd->update_every);
590
- }
620
cd->pid = 0;
592
-
621
if(unlikely(!cd->enabled)) break;
622
}
623