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: 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
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
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: 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
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 2003, 2004, 2005 */ + +#include "gw-config.h" #include #include @@ -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_statu
[PATCH] configurable max retries
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 2003, 2004, 2005 */ + +#include "gw-config.h" #include #include @@ -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 && bb_status != BB_DEAD) { +debug("bb.sms", 0, "re-queing SMS not-yet-to-be resent"); +gwlist_produce(outg