Hi Christopher, Willy,
Op 7-12-2017 om 19:33 schreef Willy Tarreau:
On Thu, Dec 07, 2017 at 04:27:16PM +0100, Christopher Faulet wrote:
Honestly, I don't know which version is the best.
Just let me know guys :-)
imho Christopher's patch is smaller and probably easier to maintain and
eventually remove without adding (unneeded) code to the
set_server_check_status(). Though it is a bit less obvious to me that it
will have the same effect, i works just as well.
Email alerts should
probably be rewritten to not use the checks. This was the only solution to
do connections by hand when Simon implemented it. That's not true anymore.
I agree and I think I was the one asking Simon to do it like this by then
eventhough he didn't like this solution. That was an acceptable tradeoff
in my opinion, with very limited impact on existing code. Now with applets
being much more flexible we could easily reimplement a more complete and
robust SMTP engine not relying on hijacking the tcp-check engine anymore.
Willy
A 'smtp engine' for sending email-alert's might be nice eventually but
that is not easily done 'today'. (not by me anyhow) (Would it group
messages together if multiple are created within a short time-span?)
As for the current issue / patch, i prefer the solution Christopher
found/made.
Made a new version of it with a bit of extra comments inside the code,
removed a unrelated white-space change, and added a matching patch
description.
Or perhaps Christopher can create it under his own name? Either way is
fine for me. :)
Regards,
PiBa-NL / Pieter
From 3129e1ae21e41a026f6d067b3658f6643835974c Mon Sep 17 00:00:00 2001
From: PiBa-NL <pba_...@yahoo.com>
Date: Wed, 6 Dec 2017 01:35:43 +0100
Subject: [PATCH] BUG/MEDIUM: email-alert: don't set server check status from a
email-alert task
This avoids possible 100% cpu usage deadlock on a EMAIL_ALERTS_LOCK and avoids
sending lots of emails when 'option log-health-checks' is used. It is avoided
to change the server state and possibly queue a new email while
processing the email alert by setting check->status to HCHK_STATUS_UNKNOWN
which will exit the set_server_check_status(..) early.
This needs to be backported to 1.8.
---
src/checks.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/checks.c b/src/checks.c
index eaf84a2..3a6f020 100644
--- a/src/checks.c
+++ b/src/checks.c
@@ -3145,7 +3145,7 @@ static struct task *process_email_alert(struct task *t)
t->expire = now_ms;
check->server = alert->srv;
check->tcpcheck_rules = &alert->tcpcheck_rules;
- check->status = HCHK_STATUS_INI;
+ check->status = HCHK_STATUS_UNKNOWN; // the
UNKNOWN status is used to exit set_server_check_status(.) early
check->state |= CHK_ST_ENABLED;
}
--
2.10.1.windows.1
