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

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