Re: [PATCH] configurable max retries
Hi Stipe,
it's pretty simple why we need 2 ints. We need to track 2 values: a)
time of last resend, in order to not to send too oft (delayed); b) how
much retries we did.
If you know how to awoid or better to say how track 2 values within one
int I would like to hear.
Thanks,
Alex
Stipe Tolj wrote:
Stipe Tolj wrote:
+1 on the logic, I assume you did regression test this Alex, did you?
I'm -1 on the new config directive naming, and I'd like to change it to:
sms-resend-freq
sms-resend-retry
Reasons:
'sms-resend-freq' follows naming convention as introduced to prior
config directives 'store-dump-freq' within core group and 'timer-freq'
for wapbox group
'sms-resend-retry' follows naming and semantical convention introduced
via 'http-request-retry' within smsbox group.
I'll change these and commit to cvs.
commited the naming convention fix for the two new directives to cvs.
@Alex: are we required in having two seperate integers in the Msg struct
to reflect the resending issue? I know that you generally do reduce
needed vars as much as possible, so a short description why two are
needed would be great.
Stipe
mailto:stolj_{at}_wapme-group.de
---
Wapme Systems AG
Vogelsanger Weg 80
40470 Düsseldorf, NRW, Germany
phone: +49.211.74845.0
fax: +49.211.74845.299
mailto:info_{at}_wapme-systems.de
http://www.wapme-systems.de/
---
Re: [PATCH] configurable max retries
Alexander Malysh wrote:
Hi Stipe,
it's pretty simple why we need 2 ints. We need to track 2 values: a)
time of last resend, in order to not to send too oft (delayed); b) how
much retries we did.
If you know how to awoid or better to say how track 2 values within one
int I would like to hear.
now, obviously via bit operations ;) ... but that's not considered serios here,
or?
Stipe
mailto:stolj_{at}_wapme-group.de
---
Wapme Systems AG
Vogelsanger Weg 80
40470 Düsseldorf, NRW, Germany
phone: +49.211.74845.0
fax: +49.211.74845.299
mailto:info_{at}_wapme-systems.de
http://www.wapme-systems.de/
---
Re: [PATCH] configurable max retries
hmm, time_t as bit operations... Good luck ;)
Thanks,
Alex
Stipe Tolj wrote:
Alexander Malysh wrote:
Hi Stipe,
it's pretty simple why we need 2 ints. We need to track 2 values: a)
time of last resend, in order to not to send too oft (delayed); b) how
much retries we did.
If you know how to awoid or better to say how track 2 values within
one int I would like to hear.
now, obviously via bit operations ;) ... but that's not considered
serios here, or?
Stipe
mailto:stolj_{at}_wapme-group.de
---
Wapme Systems AG
Vogelsanger Weg 80
40470 Düsseldorf, NRW, Germany
phone: +49.211.74845.0
fax: +49.211.74845.299
mailto:info_{at}_wapme-systems.de
http://www.wapme-systems.de/
---
Re: [PATCH] configurable max retries
Alexander Malysh wrote:
Index: gwlib/cfg.def
===
RCS file: /home/cvs/gateway/gwlib/cfg.def,v
retrieving revision 1.112
diff -a -u -p -r1.112 cfg.def
--- gwlib/cfg.def21 Sep 2005 02:01:22 -1.112
+++ gwlib/cfg.def26 Oct 2005 20:36:52 -
@@ -118,6 +118,8 @@ SINGLE_GROUP(core,
OCTSTR(dlr-storage)
OCTSTR(maximum-queue-length)
OCTSTR(sms-incoming-queue-limit)
+OCTSTR(sms-resend-frequency)
+OCTSTR(sms-max-resend-retry)
)
+1 on the logic, I assume you did regression test this Alex, did you?
I'm -1 on the new config directive naming, and I'd like to change it to:
sms-resend-freq
sms-resend-retry
Reasons:
'sms-resend-freq' follows naming convention as introduced to prior config
directives 'store-dump-freq' within core group and 'timer-freq' for wapbox group
'sms-resend-retry' follows naming and semantical convention introduced via
'http-request-retry' within smsbox group.
I'll change these and commit to cvs.
Stipe
mailto:stolj_{at}_wapme-group.de
---
Wapme Systems AG
Vogelsanger Weg 80
40470 Düsseldorf, NRW, Germany
phone: +49.211.74845.0
fax: +49.211.74845.299
mailto:info_{at}_wapme-systems.de
http://www.wapme-systems.de/
---
Re: [PATCH] configurable max retries
Stipe Tolj wrote:
+1 on the logic, I assume you did regression test this Alex, did you?
I'm -1 on the new config directive naming, and I'd like to change it to:
sms-resend-freq
sms-resend-retry
Reasons:
'sms-resend-freq' follows naming convention as introduced to prior
config directives 'store-dump-freq' within core group and 'timer-freq'
for wapbox group
'sms-resend-retry' follows naming and semantical convention introduced
via 'http-request-retry' within smsbox group.
I'll change these and commit to cvs.
commited the naming convention fix for the two new directives to cvs.
@Alex: are we required in having two seperate integers in the Msg struct to
reflect the resending issue? I know that you generally do reduce needed vars as
much as possible, so a short description why two are needed would be great.
Stipe
mailto:stolj_{at}_wapme-group.de
---
Wapme Systems AG
Vogelsanger Weg 80
40470 Düsseldorf, NRW, Germany
phone: +49.211.74845.0
fax: +49.211.74845.299
mailto:info_{at}_wapme-systems.de
http://www.wapme-systems.de/
---
Re: [PATCH] configurable max retries
Hi,
as no objections were here, commited to cvs.
Thanks,
Alex
Alexander Malysh wrote:
Hi all,
please find attached patch that adds configurable max retries and resend
frequency for the temporarily failed messages to the core (means works
with all SMSC modules). Patch also includes changes to user guide for
how to use it.
Please give me feedback...
Thanks,
Alex
Index: gw/bb_smscconn.c
===
RCS file: /home/cvs/gateway/gw/bb_smscconn.c,v
retrieving revision 1.84
diff -a -u -p -r1.84 bb_smscconn.c
--- gw/bb_smscconn.c26 Oct 2005 19:04:56 - 1.84
+++ gw/bb_smscconn.c26 Oct 2005 20:36:52 -
@@ -63,7 +63,10 @@
* routing, writing actual access logs, handling failed messages etc.
*
* Kalle Marjola 2000 for project Kannel
+ * Alexander Malysh amalysh at kannel.org 2003, 2004, 2005
*/
+
+#include gw-config.h
#include errno.h
#include stdlib.h
@@ -117,6 +120,10 @@ static regex_t *black_list_regex;
static long router_thread = -1;
+/* message resend */
+static long sms_resend_frequency;
+static long sms_max_resend_retry;
+
/*
* Counter for catenated SMS messages. The counter that can be put into
* the catenated SMS message's UDH headers is actually the lowest 8 bits.
@@ -264,11 +271,32 @@ void bb_smscconn_send_failed(SMSCConn *c
}
switch (reason) {
-
-case SMSCCONN_FAILED_SHUTDOWN:
case SMSCCONN_FAILED_TEMPORARILY:
- gwlist_produce(outgoing_sms, sms);
- break;
+/*
+ * Check if SMSC link alive and if so increase resend_try and set
resend_time.
+ * If SMSC link is not active don't increase resend_try and don't set
resend_time
+ * because we don't want to delay messages because of connection
broken.
+ */
+ if (conn smscconn_status(conn) == SMSCCONN_ACTIVE) {
+/*
+ * Check if sms_max_resend_retry set and this msg has exceeded a
limit also
+ * honor single shot with sms_max_resend_retry set to zero.
+ */
+ if (sms_max_resend_retry = 0 sms-sms.resend_try =
sms_max_resend_retry) {
+ warning(0, Maximum retries for message exceeded, discarding
it!);
+ bb_smscconn_send_failed(NULL, sms, SMSCCONN_FAILED_DISCARDED,
octstr_create(Retries Exceeded));
+ break;
+ }
+ sms-sms.resend_try = (sms-sms.resend_try 0 ?
sms-sms.resend_try++ : 1);
+ time(sms-sms.resend_time);
+ }
+ gwlist_produce(outgoing_sms, sms);
+ break;
+
+case SMSCCONN_FAILED_SHUTDOWN:
+gwlist_produce(outgoing_sms, sms);
+break;
+
default:
/* write NACK to store file */
store_save_ack(sms, ack_failed);
@@ -294,7 +322,9 @@ void bb_smscconn_send_failed(SMSCConn *c
bb_smscconn_receive(conn, dlrmsg);
}
}
- msg_destroy(sms);
+
+msg_destroy(sms);
+break;
}
octstr_destroy(reply);
@@ -424,27 +454,40 @@ static void sms_router(void *arg)
ret = 0;
while(bb_status != BB_DEAD) {
-
if (newmsg == startmsg) {
- if (ret != 1) {
- debug(bb.sms, 0, sms_router: time to sleep);
- gwthread_sleep(600.0); /* hopefully someone wakes us up */
- debug(bb.sms, 0, sms_router: gwlist_len = %ld,
- gwlist_len(outgoing_sms));
- }
- startmsg = gwlist_consume(outgoing_sms);
- newmsg = NULL;
- msg = startmsg;
- } else {
- newmsg = gwlist_consume(outgoing_sms);
- msg = newmsg;
- }
- /* debug(bb.sms, 0, sms_router: handling message (%p vs %p),
-* newmsg, startmsg); */
-
- if (msg == NULL)
+if (ret != 1) {
+/*
+ * In order to reduce amount of msgs to send we sleep only the
half of frequency time
+ * but at least 1 second.
+ */
+double sleep_time = (sms_resend_frequency / 2 1 ?
sms_resend_frequency / 2 : sms_resend_frequency);
+debug(bb.sms, 0, sms_router: time to sleep %.2f secs.,
sleep_time);
+gwthread_sleep(sleep_time);
+debug(bb.sms, 0, sms_router: gwlist_len = %ld,
gwlist_len(outgoing_sms));
+}
+startmsg = msg = gwlist_consume(outgoing_sms);
+newmsg = NULL;
+}
+else {
+newmsg = msg = gwlist_consume(outgoing_sms);
+}
+
+/* shutdown ? */
+if (msg == NULL)
break;
+debug(bb.sms, 0, sms_router: handling message (%p vs %p),
+ msg, startmsg);
+
+/* handle delayed msgs */
+if (msg-sms.resend_try 0 difftime(time(NULL), msg-sms.resend_time)
sms_resend_frequency
+bb_status != BB_SHUTDOWN
