Re: Limit SMS Sending Retries/Handshake Error Patch

Hi,

Am 13.04.2010 um 14:26 schrieb hisham malik:

> On Tue, Apr 13, 2010 at 1:57 AM, Alexander Malysh <[email protected]> wrote:
>> Hi,
>> 
>> Am 02.04.2010 um 16:12 schrieb hisham malik:
>> 
>>> Attaching more patches for the kannel source.
>>> 
>>> 1. Limit SMS Sending Retries:
>>> Limit SMS sending retries in split/long messages
>> 
>> this is not a proper fix because you end to failing the whole multipart message if one part can't be send
>> e.g. due to the throttling error and this patch doesn't handle resend via the same SMSC.
> 
> Your observation is correct, but IMHO is not the common case (and
> system should be optimized for common case). In our usage, multi-part
> message almost always fail almost always fail when user's mailbox
> becomes full and cannot accept anymore messages. In this case, the
> existing kannel code keeps on retrying the failed multi-part messages
> without respecting the 'sms_resend_retry' parameter. The situation
> aggravates when there are multiple multi-part messages that failed due
> to the mailbox full issue, and the overall throughput of kannel starts
> to decrease since these failed parts are getting priority over new
> messages coming in (due to recursive nature of the code), and the
> userbase starts experiencing delays in message delivery.

unfortunately this is common case for you but not for all and we trying to do it right
for all...

> 
> So given the existing architecture of kannel, failing the whole
> multi-part message and not resending via the same SMSC is a very small
> price to pay for optimizing for the common case, and ensuring that bad
> messages do not stall the servicing of rest of the messages.
> 
> 
>> 
>>> 
>>> 2. Handshake Error and Garbage Character in Long Message:
>>> Added send_failed call to the message sending function when error is received during handshake.
>>> Fixed garbage characters in multi-part messages.
>> 
>> This patch is partially commited. I don't understand part about garbage chars. Could you please explain in details
>> why we need this patch? If we need this patch then len calculation is somewhere broken.
>> 
> I created this patch almost a year back, and can't seem to find myself
> how the garbage chars was handled. Basically the issue was in the case
> of multi-part messages. There were some bad characters that were read
> out when converting from 7-bit encoding. A patch was added to remove
> those garbage characters.

could you please retest it with current SVN version? We commited patch that fixes 7bit encoding and
should fix this issue as well.

> As for the other 'handshake error' part
> (which is part of the submitted patch), there was an else case missing
> which would handle an error response sent out when the initial command
> is sent to the handset.

yep, I commited this part of your patch. Thanks!

> 
>> Please don't use // as comment because it not valid C and if you make patches please make it with ignore white spaces
>> because it's very hard to read...
>> 
> My bad. I'll create the patches to be more readable next time around.
> 
> Thanks.
>> Thanks,
>> Alexander Malysh
>> 
>> 

Thanks,
Alexander Malysh