Re: sms-resend-* bug

Any comments?

On 8/30/06, Dziugas Baltrunas <[email protected]> wrote:
> Hi list,
>
> this is somehow strange that bug which currently floods SMSCs hasn't
> been solved for months.
>
> Ok, back to the proposed solution. What's bad with the one below?
>
> ///
> diff -u -r1.88 bb_smscconn.c
> --- bb_smscconn.c       14 May 2006 00:47:24 -0000      1.88
> +++ bb_smscconn.c       30 Aug 2006 13:10:38 -0000
> @@ -264,7 +264,7 @@
>
> void bb_smscconn_send_failed(SMSCConn *conn, Msg *sms, int reason,
> Octstr *reply)
> {
> -    if (sms->sms.split_parts != NULL) {
> +    if (sms->sms.split_parts != NULL && reason !=
> SMSCCONN_FAILED_TEMPORARILY) {
>         handle_split(conn, sms, reason);
>         octstr_destroy(reply);
>         return;
> ///
>
> I tested it and it works - sms-resend-retry and sms-resend-freq
> figures are taken into account as appropriate. What's bad with this
> patch? I don't know. bb_smscconn.c:handle_split() has the following
> code:
>
> ///
>    /*
>     * 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;
>    }
> ///
>
> Why should we try again immediately (or actually forever)? To be
> "friendly" and try to deliver all the split parts in the shot? Well,
> the result is often contrary - single split message falls in the
> never-ending loop (or actually when SMSC accepts it instead spawning
> Message Queue Full or similar) and all newly arrived messages are
> waiting in the outgoing_sms queue. Does it weight out or at least
> compete with single attempt to deliver single concatenated message
> friendly? I guess it is not.
>
> I'm looking forward to other thoughts ASAP and if the proposed way
> fits, I will remove part from handle_split() quoted above and prepare
> a patch.
>
> Regards,
> Dziugas Baltrunas
>
> On 7/21/06, Tomas Varaneckas <[email protected]> wrote:
> > Hi all,
> >
> > this problem seems to be untouched for a while. I was wondering if
> > anything is done with this subject..
> >
> > http://mantis.kannel.org/view.php?id=352
> >
> > Thanks,
> > Tomas
> >
> >
>


-- 
Dziugas