Re: [PATCH] fix bug #529 (sms-resend-* ignored for concatenated messages)

Actually, now that i'm rereading this conversation again...

> You have to remember SMSC pointer not only SMSC-name/id and then teach all
routing parts
> to respect it...

What if kannel gets shut down and then starts up again?
Obviousely, smsc pointers are different...

I say lets make smsc-id's a must and unique, that'll make things better ;)

On Thu, Mar 18, 2010 at 11:00 AM, Alexander Malysh <[email protected]>wrote:

> Hi,
>
> the possibility for endless loop is equal for each part. So this will not
> help... We need real fix...
>
> Thanks,
> Alexander Malysh
>
> Am 17.03.2010 um 18:04 schrieb Michael Zervakis:
>
> > Dear Alex,
> >
> >   From my experience endless loops from temporary errors usually occur
> before ever transmitting the first message part. It's a very rare occurence
> to transmit first part and then enter in an endless loop with left parts. A
> partial fix could be to keep two counters in split_parts struct, parts_left
> and parts_len, to make it possible to determine which part we are
> transmitting. So if temp error occurs and we are sending first part we can
> safely put the whole message into resend queue without the drawback of
> sending extra SMS parts to receiver. I know this is not a real fix, but I
> believe it will hanlde most of the endless loop cases.
> >
> >
> > Am 17.12.2009 um 11:28 schrieb Konstantin Vayner:
> >
> >
> >
> > why remembering smsc-id in sms.smsc_id is not enough?
> >
> >
> > due to accepted-smsc in smsc config group and because SMSCs may have the
> same names (or even no names defines)...
> >
> >
> >
> > 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]<mailto:
> [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]<mailto:
> [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>
> >
> >
> >
> >
> >
> >
> >
>
>