Pieter,

I'm just seeing this part in your description while merging
the patch :

On Sun, Nov 08, 2015 at 07:19:21PM +0100, PiBa-NL wrote:
> >>>HOWEVER.
> >>>-i have not checked for memoryleaks, sockets not being closed properly
> >>>(i dont know how to..)

I'm not worried for this one, at first glance it looks OK.

> >>>-is setting current and last steps to null the proper way to reset the
> >>>step of rule evaluation?

Yes that's apparently it.

> >>>-CO_FL_ERROR is set when there is a connection error.. this seems to be
> >>>the proper check.

Indeed.

> >>>-but check->conn->flags & 0xFF  is a bit of s guess from observing the
> >>>flags when it could connect but the server did not respond 
> >>>properly.. is there a other better way?

This one is ugly. First, you should never use the numeric values
when there are defines or enums because if these flags happen to
change or move to something else, your value will not be spotted
and will not be converted.

Second, normally you would just need "!CONNECTED", as this flag
is set once the connection is established. Note that I understood
from the commit message that you wanted to cover from connection
issues, but the code makes me think that you wanted to retry even
if the connection was properly established but the mail could not
be completely delivered.

Thus I'm attaching two proposals that I'd like you to test, the
first one verifies if the connection was established or not. The
second one checks if we've reached the last rule (implying all
of them were executed).

Thanks,
Willy

>From 8b2520d15340f96e6e7934870600fd80c86a8942 Mon Sep 17 00:00:00 2001
From: Pieter Baauw <piba.nl....@gmail.com>
Date: Sun, 26 Jul 2015 20:47:27 +0200
Subject: MEDIUM: mailer: retry sending a mail up to 3 times

Currently only 1 connection attempt (syn packet) was send, this patch
increases that to 3 attempts. This to make it less likely the mail is
lost due to a single lost packet.
---
 src/checks.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/src/checks.c b/src/checks.c
index e77926a..32ff983 100644
--- a/src/checks.c
+++ b/src/checks.c
@@ -1408,7 +1408,7 @@ static struct task *server_warmup(struct task *t)
  *
  * It can return one of :
  *  - SF_ERR_NONE if everything's OK and tcpcheck_main() was not called
- *  - SF_ERR_UP if if everything's OK and tcpcheck_main() was called
+ *  - SF_ERR_UP if everything's OK and tcpcheck_main() was called
  *  - SF_ERR_SRVTO if there are no more servers
  *  - SF_ERR_SRVCL if the connection was refused by the server
  *  - SF_ERR_PRXCOND if the connection has been limited by the proxy (maxconn)
@@ -3065,6 +3065,7 @@ static struct task *process_email_alert(struct task *t)
                        LIST_DEL(&alert->list);
 
                        check->state |= CHK_ST_ENABLED;
+                       check->fall = 0;
                }
 
        }
@@ -3074,6 +3075,16 @@ static struct task *process_email_alert(struct task *t)
        if (!(check->state & CHK_ST_INPROGRESS) && check->tcpcheck_rules) {
                struct email_alert *alert;
 
+               if ((check->conn->flags & CO_FL_ERROR) || // connection failed, 
try again
+                   !(check->conn->flags & CO_FL_CONNECTED)) { // did not 
manage to connect, try again
+                       if (check->fall < 2) {     // 3 attempts max
+                               check->current_step = NULL;
+                               check->last_started_step = NULL;
+                               check->fall++;
+                               return t;
+                       }
+               }
+
                alert = container_of(check->tcpcheck_rules, typeof(*alert), 
tcpcheck_rules);
                email_alert_free(alert);
 
-- 
1.7.12.2.21.g234cd45.dirty

>From 1601982888b700e454f4cb8bfb436048a3e8d71d Mon Sep 17 00:00:00 2001
From: Pieter Baauw <piba.nl....@gmail.com>
Date: Sun, 26 Jul 2015 20:47:27 +0200
Subject: MEDIUM: mailer: retry sending a mail up to 3 times

Currently only 1 connection attempt (syn packet) was send, this patch
increases that to 3 attempts. This to make it less likely the mail is
lost due to a single lost packet.
---
 src/checks.c | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/src/checks.c b/src/checks.c
index e77926a..98011ec 100644
--- a/src/checks.c
+++ b/src/checks.c
@@ -1408,7 +1408,7 @@ static struct task *server_warmup(struct task *t)
  *
  * It can return one of :
  *  - SF_ERR_NONE if everything's OK and tcpcheck_main() was not called
- *  - SF_ERR_UP if if everything's OK and tcpcheck_main() was called
+ *  - SF_ERR_UP if everything's OK and tcpcheck_main() was called
  *  - SF_ERR_SRVTO if there are no more servers
  *  - SF_ERR_SRVCL if the connection was refused by the server
  *  - SF_ERR_PRXCOND if the connection has been limited by the proxy (maxconn)
@@ -3065,6 +3065,7 @@ static struct task *process_email_alert(struct task *t)
                        LIST_DEL(&alert->list);
 
                        check->state |= CHK_ST_ENABLED;
+                       check->fall = 0;
                }
 
        }
@@ -3074,6 +3075,18 @@ static struct task *process_email_alert(struct task *t)
        if (!(check->state & CHK_ST_INPROGRESS) && check->tcpcheck_rules) {
                struct email_alert *alert;
 
+               if ((check->conn->flags & CO_FL_ERROR) || // connection failed, 
try again
+                   !(check->conn->flags & CO_FL_CONNECTED) || // did not 
manage to connect, try again
+                   (check->current_step &&
+                    &check->current_step->list != check->tcpcheck_rules)) { // 
aborted before last rule, try again
+                       if (check->fall < 2) {     // 3 attempts max
+                               check->current_step = NULL;
+                               check->last_started_step = NULL;
+                               check->fall++;
+                               return t;
+                       }
+               }
+
                alert = container_of(check->tcpcheck_rules, typeof(*alert), 
tcpcheck_rules);
                email_alert_free(alert);
 
-- 
1.7.12.2.21.g234cd45.dirty

Reply via email to