Hi, as I already wrote, this patch is not enough. Please see ML wy...
Thanks, Alexander Malysh Am 09.09.2010 um 19:04 schrieb Michael Zervakis: > Hi, > > This patch is a possible solution fog Bug #529. The Msg definition was > modified so we can store the conn->id originally selected by router. > > SMSC-A -> splits (2 parts) -> 1 part sent OK -> 2 part get temp. error -> > write the conn->id and put it into global queue for resend -> router reads > msg->sms.msg_ConnId and sends failed part via same connection (if still > active) > > > Regards, > > > From: [email protected] [mailto:[email protected]] On Behalf Of > Konstantin Vayner > Sent: Thursday, December 17, 2009 12:28 PM > To: Alexander Malysh > Cc: [email protected] > Subject: Re: [PATCH] fix bug #529 (sms-resend-* ignored for concatenated > messages) > > > why remembering smsc-id in sms.smsc_id is not enough? > how does smsbox remember routing when i submit a message with predefined smsc > id from http (sendsms) ? > > On Thu, Dec 17, 2009 at 12:10 PM, Alexander Malysh <[email protected]> wrote: > > > Am 17.12.2009 um 10:43 schrieb Konstantin Vayner: > > > > so the best option would be to requeue the part via same smsc, right ? > > > yes, but it's not easy todo. You have to remember SMSC pointer not only > SMSC-name/id and then teach all routing parts > > to respect it... > > > > cause requeueing all parts may also get extra messages to the handset despite > it not being able to reconstruct (not to mention the extra money ;) ) > > On Thu, Dec 17, 2009 at 11:33 AM, Alexander Malysh <[email protected]> wrote: > > Hi, > > > unfortunately this will not work as expected (the rule is: _all_ parts if > multipart message have to be send via the same SMSC)... > > > example: > > > SMSC-A -> splits (2 parts) -> 1 part sent OK -> 2 part get temp. > error -> you put it into global queue for resend -> 2 part sent via SMSC-B -> > handset rejects it > > > We have only two possibility here: > > 1) if temp error occurs put the _whole_ message into resend queue and resend > then _all_ parts (very easy todo) > > 2) remember smsc which was used for first parts and resend it via the same > smsc (complicated but save money :) ) > > > Thanks, > > Alexander Malysh > > > Am 16.12.2009 um 18:17 schrieb Konstantin Vayner: > > > > Bug report: http://redmine.kannel.org/issues/show/529 > > Quote from gw/bb_smscconn.c : > > static void handle_split(SMSCConn *conn, Msg *msg, long reason) > { > struct split_parts *split = msg->sms.split_parts; > > /* > * If temporarely failed, try again immediately but only if connection > active. > * Because if connection is not active we will loop for ever here consuming > 100% CPU > * time due to internal queue cleanup in smsc module that call > bb_smscconn_failed. > */ > if (reason == SMSCCONN_FAILED_TEMPORARILY && smscconn_status(conn) == > SMSCCONN_ACTIVE && > smscconn_send(conn, msg) == 0) { > /* destroy this message because it will be duplicated in smsc module */ > msg_destroy(msg); > return; > } > > (end quote) > > So, if an smsc is alive and throws temporary error every time you try to > submit such a message, we enter endless loop of attempting to resend it.... > > > Suggested patch follows (also attached). > Sorry its not cvs diff - having firewall issues accessing pserver now so i > ran diff vs snapshot generated yesterday > I will be able to produce a normal cvs diff tomorrow morning if it is needed > > > --- kannel-snapshot/gw/bb_smscconn.c 2009-11-15 16:12:28.000000000 +0200 > +++ gateway-cvs/gw/bb_smscconn.c 2009-12-16 19:47:32.000000000 +0200 > @@ -203,18 +203,6 @@ > struct split_parts *split = msg->sms.split_parts; > /* > - * If temporarely failed, try again immediately but only if connection > active. > - * Because if connection is not active we will loop for ever here > consuming 100% CPU > - * time due to internal queue cleanup in smsc module that call > bb_smscconn_failed. > - */ > - if (reason == SMSCCONN_FAILED_TEMPORARILY && smscconn_status(conn) == > SMSCCONN_ACTIVE && > - smscconn_send(conn, msg) == 0) { > - /* destroy this message because it will be duplicated in smsc module > */ > - msg_destroy(msg); > - return; > - } > - - /* > * if the reason is not a success and status is still success > * then set status of a split to the reason. > * Note: reason 'malformed','discarded' or 'rejected' has higher priority! > @@ -303,7 +291,7 @@ > void bb_smscconn_send_failed(SMSCConn *conn, Msg *sms, int reason, Octstr > *reply) > { > - if (sms->sms.split_parts != NULL) { > + if (reason != SMSCCONN_FAILED_TEMPORARILY && sms->sms.split_parts != > NULL) { > handle_split(conn, sms, reason); > octstr_destroy(reply); > return; > > > > > > <bb_smscconn.diff> > > > > > > Index: gw/bb_smscconn.c > =================================================================== > --- gw/bb_smscconn.c (revision 4838) > +++ gw/bb_smscconn.c (working copy) > @@ -207,11 +207,34 @@ > * Because if connection is not active we will loop for ever here > consuming 100% CPU > * time due to internal queue cleanup in smsc module that call > bb_smscconn_failed. > */ > - if (reason == SMSCCONN_FAILED_TEMPORARILY && smscconn_status(conn) == > SMSCCONN_ACTIVE && > - smscconn_send(conn, msg) == 0) { > + /*if (reason == SMSCCONN_FAILED_TEMPORARILY && smscconn_status(conn) == > SMSCCONN_ACTIVE && > + smscconn_send(conn, msg) == 0) { */ > /* destroy this message because it will be duplicated in smsc module > */ > - msg_destroy(msg); > + /* msg_destroy(msg); > return; > + }*/ > + > + > + > + if (reason == SMSCCONN_FAILED_TEMPORARILY && smscconn_status(conn) == > SMSCCONN_ACTIVE) > + { > + if (sms_resend_retry >= 0 && msg->sms.resend_try >= sms_resend_retry) > /*check how many times the split message has been sent */ > + { > + warning(0, "SMPP[%s] :Maximum retries for message [%s] exceeded > resend %ld times, discarding it!", octstr_get_cstr(conn->id), > octstr_get_cstr(msg->sms.msgdata),msg->sms.resend_try); > + handle_split(conn, msg, SMSCCONN_FAILED_DISCARDED); > + /* msg_destroy(msg); */ > + return; > + } > + > + warning(0, "SMPP[%s] :Split message failed temporarily sending back > to sms_router", octstr_get_cstr(conn->id)); > + > + msg->sms.resend_try = (msg->sms.resend_try > 0 ? msg->sms.resend_try > + 1 : 1); > + debug("bb.sms.splits", 0, "SMPP[%s] :Resend counter of message set to > %ld", octstr_get_cstr(conn->id),msg->sms.resend_try); > + time(&msg->sms.resend_time); > + msg->sms.msg_ConnId = octstr_duplicate(conn->id); > + > + gwlist_produce(outgoing_sms, msg); /* write it back to global queue > */ > + return; > } > > /* > @@ -1229,6 +1252,28 @@ > bp_load = bo_load = queue_length = 0; > > conn = NULL; > + /* if msg was a failed part then route to the defined SMSC connection */ > + if (msg->sms.msg_ConnId != NULL) > + { > + for (i=0; i < gwlist_len(smsc_list); i++) > + { > + conn = gwlist_get(smsc_list, i); > + smscconn_info(conn, &info); > + queue_length += (info.queued > 0 ? info.queued : 0); > + > + if(octstr_compare(msg->sms.msg_ConnId, conn->id) == 0) > + { > + if((smscconn_usable(conn,msg) != -1) && (info.status == > SMSCCONN_ACTIVE && info.queued < max_queue)) > + { > + best_preferred = conn; > + debug("bb.sms",0,"Message with pre-set connection to > SMSC[%s] found",octstr_get_cstr(msg->sms.msg_ConnId)); > + continue; > + } > + } > + } > + } > + else > + { > for (i=0; i < gwlist_len(smsc_list); i++) { > conn = gwlist_get(smsc_list, (i+s) % gwlist_len(smsc_list)); > > @@ -1265,6 +1310,8 @@ > bo_load = info.load; > } > } > + } > + > queue_length += gwlist_len(outgoing_sms); > if (max_outgoing_sms_qlength > 0 && !resend && > queue_length > gwlist_len(smsc_list) * max_outgoing_sms_qlength) { > Index: gw/msg-decl.h > =================================================================== > --- gw/msg-decl.h (revision 4838) > +++ gw/msg-decl.h (working copy) > @@ -85,6 +85,7 @@ > OCTSTR(msgdata) > INTEGER(time) > OCTSTR(smsc_id) > + OCTSTR(msg_ConnId); /* field required to reroute concatenated Msg > parts via the same SMSC connection. Bug 529*/ > OCTSTR(smsc_number) > OCTSTR(foreign_id) > OCTSTR(service)
