Forgot to include list, sorry. And then the attachment dropped of.. Resending.
Op 8-11-2015 om 17:33 schreef PiBa-NL:
Hi Ben, Willy, Simon,

Ben, thanks for the review.
Hoping 'release pressure' has cleared for Willy i'm resending the patch now, with with your comments incorporated.

CC, to Simon as maintainer of mailers part so he can give approval (or not).

The original reservations i had when sending this patch still apply. See the "HOWEVER." part in the bottom mail.

Hoping it might get merged to improve mailer reliability. So no 'server down' email gets lost..
Thanks everyone for your time :) .

Regards,
PiBa-NL

Op 22-9-2015 om 16:43 schreef Ben Cabot:
Hi PiBa-NL,

Malcolm has asked me to take a look at this.  While I don't know
enough to answer the questions about the the design and implementation
I have tested the patch. In my testing it works well and I have a
couple of comments.

I had a warning when building, struct email_alert *alert; should be
before process_chk(t); or gcc moans (Warning: ISO C90 forbids mixed
declarations and code).
Ive moved the stuct to the top of the if statement where it was before my patch. I expect that to fix the warning.

It makes in total 4 attempts to send the mail where I believe it should be 3?
If the total desired attempts is 3 It looks like "if (check->fall < 3)
{ " should be "if (check->fall < 2)" with "check->fall++;" inside the
if statement. I may be wrong I've only briefly looked.
Yes it did '3 retries'. Ive changed to make it a total of '3 attempts' which is more like a normal 3x SYN packet when opening a failing connection.
While testing this I've realised it would also be nice to log when the
email fails to send after 3 attempts but that is a job for another
day.

Thanks for submitting this as its helpful for us, also for helping
with my patch.  I am still waiting for Willy to come back to me about
mine as well. As he is in the middle of a release I expect he is very
busy at the moment so I'll wait a while before giving him a poke and
following up. Hopefully I've been of some help to you.
Thanks for testing!
Kind Regards,
Ben

On 4 August 2015 at 20:35, PiBa-NL <piba.nl....@gmail.com> wrote:
bump?
-------- Doorgestuurd bericht --------
Onderwerp: request for comment - [PATCH] MEDIUM: mailer: retry sending
a mail up to 3 times
Datum:  Sun, 26 Jul 2015 21:08:41 +0200
Van:    PiBa-NL <piba.nl....@gmail.com>
Aan:    HAproxy Mailing Lists <haproxy@formilux.org>



Hi guys,

Ive created a small patch that will retry sending a mail 3 times if it
fails the first time.
Its seems to work in my limited testing..

HOWEVER.
-i have not checked for memoryleaks, sockets not being closed properly
(i dont know how to..)
-is setting current and last steps to null the proper way to reset the
step of rule evaluation?
-CO_FL_ERROR is set when there is a connection error.. this seems to be
the proper check.
-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?
-i used the 'fall' variable to track the number of retries.. should i
have created a separate 'retries' variable?

Thanks for any feedback you can give me.

Best regards,
PiBa-NL







From 18fd2740b7c9f511e03afe9ebb8237f6a640a141 Mon Sep 17 00:00:00 2001
From: Pieter Baauw <piba.nl....@gmail.com>
Date: Sun, 26 Jul 2015 20:47:27 +0200
Subject: [PATCH] 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 them mail is lost due to a 
single lost packet.
---
 src/checks.c | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/src/checks.c b/src/checks.c
index e77926a..335eb9a 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,17 @@ 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 & 0xFF) // did not reach the 
'normal end', try again
+                       ) {
+                       if (check->fall < 2) {
+                               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.9.5.msysgit.1

Reply via email to