Hi Simon,

Ok, ill try and see if i can add a config setting for it. And move the whitespace changes to a separate patch.
Will take me some time..

Thanks for your feedback,
PiBa-NL

Op 21-11-2015 om 0:30 schreef Simon Horman:
On Fri, Nov 20, 2015 at 11:58:19PM +0100, PiBa-NL wrote:
Hi Willy,

Op 16-11-2015 om 19:57 schreef Willy Tarreau:
I agree with you since we don't know the timeout value nor what it applies
to (connection or anything). Thus I think that we should first find and
change that value, and maybe after that take your patch to permit real
retries in case of connection failures.

Thanks,
Willy
Alright found the timeout, which was actually marked with a rather clear
'warning message'.
Perhaps attached patch could be applied to increase that timeout to 10
seconds? Should a similar 'warning' still be added to say this allows for
some retransmits? (In my test it sends 4 SYN packets if it cannot connect at
all.)

Or should it be approached  in a different way? Perhaps as a configuration
option on the mailers section?
I think it should be configurable, the mailers section sounds logical to me.

Thanks,
PiBa-NL
>From eaf95bea0af6aa3b553a6e038997b5d339b507da Mon Sep 17 00:00:00 2001
From: Pieter Baauw <piba.nl....@gmail.com>
Date: Fri, 20 Nov 2015 23:39:48 +0100
Subject: [PATCH] mailer: set longer timeout on mail alert to allow for a few
  tcp retransmits

---
  include/common/defaults.h | 9 +++++----
  src/checks.c              | 4 +---
  2 files changed, 6 insertions(+), 7 deletions(-)

diff --git a/include/common/defaults.h b/include/common/defaults.h
index d1994e8..b0d7c07 100644
--- a/include/common/defaults.h
+++ b/include/common/defaults.h
@@ -144,10 +144,11 @@
#define CONN_RETRIES 3 -#define CHK_CONNTIME 2000
-#define        DEF_CHKINTR     2000
-#define DEF_FALLTIME    3
-#define DEF_RISETIME    2
+#define CHK_CONNTIME          2000
+#define DEF_CHKINTR           2000
+#define DEF_MAILALERTTIME     10000
+#define DEF_FALLTIME          3
+#define DEF_RISETIME          2
  #define DEF_AGENT_FALLTIME    1
  #define DEF_AGENT_RISETIME    1
  #define DEF_CHECK_REQ   "OPTIONS / HTTP/1.0\r\n"
I would prefer if the whitespace changes, that is all
changes other than the line that adds DEF_MAILALERTTIME,
were not part of this patch.

diff --git a/src/checks.c b/src/checks.c
index e77926a..cecd7ed 100644
--- a/src/checks.c
+++ b/src/checks.c
@@ -3108,9 +3108,7 @@ static int init_email_alert_checks(struct server *s)
LIST_INIT(&q->email_alerts); - check->inter = DEF_CHKINTR; /* XXX: Would like to Skip to the next alert, if any, ASAP.
-                                            * But need enough time so that 
timeouts don't occur
-                                            * during tcp check procssing. For 
now just us an arbitrary default. */
+               check->inter = DEF_MAILALERTTIME;
I would prefer if the comment was retained, it is still valid.

                check->rise = DEF_AGENT_RISETIME;
                check->fall = DEF_AGENT_FALLTIME;
                err_str = init_check(check, PR_O2_TCPCHK_CHK);
--
1.9.5.msysgit.1



Reply via email to