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

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]> 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>
>> 
>> 
> 
>