The uuid would be different each time you start the service right? What would happen if Kannel is restarted and there were pending parts to send? The new uuid wouldn't match with the pending segments.
Regards, -- Alejandro Guerrieri aguerri...@kannel.org On 13/09/2010, at 19:16, Alexander Malysh wrote: > Hi, > > great idea to use uuid to uniquely identify smscconn. we can use this for > http admin as well. > > Thanks, > Alexander Malysh > > Am 13.09.2010 um 19:04 schrieb Michael Zervakis: > >> Hi, >> >> I figured out that we can use the same mechanism as in Msg struct to >> uniquelly identify each SMSCconn. In the attached patch the smscconn struct >> was modified to include a uuid_t field and Msg struct to store the smscconn >> uuid_t if we get a temporary error. Smsc_rout2 was also modified to respect >> msg->msg_ConnId if set. >> >> SMSC-A -> splits (2 parts) -> 1 part sent OK -> 2 part get temp. error -> >> write the conn->uuid_t and put it into global queue for resend -> router >> reads msg->msg_ConnId and sends failed part via same connection (if still >> active) >> >> >> -----Original Message----- >> From: Alexander Malysh [mailto:malys...@googlemail.com] On Behalf Of >> Alexander Malysh >> Sent: Sunday, September 12, 2010 1:06 PM >> To: Michael Zervakis >> Cc: devel@kannel.org; pon...@appcell.net >> Subject: Re: [PATCH] fix bug #529 (sms-resend-* ignored for concatenated >> messages) >> >> >> Hi, >> >> >> as I already wrote, this patch is not enough. Please see ML wy... >> >> >> Thanks, >> >> Alexander Malysh >> >> >> >> Am 09.09.2010 um 19:04 schrieb Michael Zervakis: >> >> >>> Hi, >> >>> >> >>> This patch is a possible solution fog Bug #529. The Msg definition was >>> modified so we can store the conn->id originally selected by router. >> >>> >> >>> SMSC-A -> splits (2 parts) -> 1 part sent OK -> 2 part get temp. error -> >>> write the conn->id and put it into global queue for resend -> router reads >>> msg->sms.msg_ConnId and sends failed part via same connection (if still >>> active) >> >>> >> >>> >> >>> Regards, >> >>> >> >>> >> >>> From: devel-boun...@kannel.org [mailto:devel-boun...@kannel.org] On Behalf >>> Of Konstantin Vayner >> >>> Sent: Thursday, December 17, 2009 12:28 PM >> >>> To: Alexander Malysh >> >>> Cc: devel@kannel.org >> >>> Subject: Re: [PATCH] fix bug #529 (sms-resend-* ignored for concatenated >>> messages) >> >>> >> >>> >> >>> why remembering smsc-id in sms.smsc_id is not enough? >> >>> 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 <amal...@kannel.org> >>> 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 <amal...@kannel.org> >>> 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> >> >>> >> >>> >> >>> >> >>> >> >>> >> >>> Index: gw/bb_smscconn.c >> >>> =================================================================== >> >>> --- gw/bb_smscconn.c (revision 4838) >> >>> +++ gw/bb_smscconn.c (working copy) >> >>> @@ -207,11 +207,34 @@ >> >>> * 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) { >> >>> + /*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); >> >>> + /* msg_destroy(msg); >> >>> return; >> >>> + }*/ >> >>> + >>> + >>> + >> >>> + if (reason == SMSCCONN_FAILED_TEMPORARILY && smscconn_status(conn) == >>> SMSCCONN_ACTIVE) >> >>> + { >> >>> + if (sms_resend_retry >= 0 && msg->sms.resend_try >= >>> sms_resend_retry) /*check how many times the split message has been sent */ >> >>> + { >>> + warning(0, "SMPP[%s] :Maximum retries for message [%s] exceeded >>> resend %ld times, discarding it!", octstr_get_cstr(conn->id), >>> octstr_get_cstr(msg->sms.msgdata),msg->sms.resend_try); >> >>> + handle_split(conn, msg, SMSCCONN_FAILED_DISCARDED); >>> + /* msg_destroy(msg); */ >> >>> + return; >> >>> + } >> >>> + >>> + warning(0, "SMPP[%s] :Split message failed temporarily sending back >>> to sms_router", octstr_get_cstr(conn->id)); >> >>> + >>> + msg->sms.resend_try = (msg->sms.resend_try > 0 ? >>> msg->sms.resend_try + 1 : 1); >> >>> + debug("bb.sms.splits", 0, "SMPP[%s] :Resend counter of message set >>> to %ld", octstr_get_cstr(conn->id),msg->sms.resend_try); >> >>> + time(&msg->sms.resend_time); >> >>> + msg->sms.msg_ConnId = octstr_duplicate(conn->id); >>> + >>> + gwlist_produce(outgoing_sms, msg); /* write it back to global queue >>> */ >> >>> + return; >> >>> } >> >>> >> >>> /* >> >>> @@ -1229,6 +1252,28 @@ >> >>> bp_load = bo_load = queue_length = 0; >> >>> >> >>> conn = NULL; >> >>> + /* if msg was a failed part then route to the defined SMSC connection >>> */ >> >>> + if (msg->sms.msg_ConnId != NULL) >> >>> + { >>> + for (i=0; i < gwlist_len(smsc_list); i++) >> >>> + { >> >>> + conn = gwlist_get(smsc_list, i); >> >>> + smscconn_info(conn, &info); >> >>> + queue_length += (info.queued > 0 ? info.queued : 0); >> >>> + >>> + if(octstr_compare(msg->sms.msg_ConnId, conn->id) == 0) >> >>> + { >>> + if((smscconn_usable(conn,msg) != -1) && (info.status == >>> SMSCCONN_ACTIVE && info.queued < max_queue)) >> >>> + { >> >>> + best_preferred = conn; >>> + debug("bb.sms",0,"Message with pre-set connection to >>> SMSC[%s] found",octstr_get_cstr(msg->sms.msg_ConnId)); >> >>> + continue; >> >>> + } >> >>> + } >> >>> + } >> >>> + } >> >>> + else >> >>> + { >> >>> for (i=0; i < gwlist_len(smsc_list); i++) { >> >>> conn = gwlist_get(smsc_list, (i+s) % gwlist_len(smsc_list)); >> >>> >> >>> @@ -1265,6 +1310,8 @@ >> >>> bo_load = info.load; >> >>> } >> >>> } >> >>> + } >> >>> + >>> queue_length += gwlist_len(outgoing_sms); >> >>> if (max_outgoing_sms_qlength > 0 && !resend && >> >>> queue_length > gwlist_len(smsc_list) * max_outgoing_sms_qlength) { >> >>> Index: gw/msg-decl.h >> >>> =================================================================== >> >>> --- gw/msg-decl.h (revision 4838) >> >>> +++ gw/msg-decl.h (working copy) >> >>> @@ -85,6 +85,7 @@ >> >>> OCTSTR(msgdata) >> >>> INTEGER(time) >> >>> OCTSTR(smsc_id) >> >>> + OCTSTR(msg_ConnId); /* field required to reroute concatenated Msg >>> parts via the same SMSC connection. Bug 529*/ >> >>> OCTSTR(smsc_number) >> >>> OCTSTR(foreign_id) >> >>> OCTSTR(service) >> >> >> Index: bb_smscconn.c >> =================================================================== >> --- bb_smscconn.c (revision 4843) >> +++ bb_smscconn.c (working copy) >> @@ -201,17 +201,46 @@ >> static void handle_split(SMSCConn *conn, Msg *msg, long reason) >> { >> struct split_parts *split = msg->sms.split_parts; >> + char id[UUID_STR_LEN + 1]; >> >> + >> /* >> * 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) { >> + /*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); >> + /* msg_destroy(msg); >> return; >> + }*/ >> + >> + >> + if (reason == SMSCCONN_FAILED_TEMPORARILY && smscconn_status(conn) == >> SMSCCONN_ACTIVE) >> + { >> + if (sms_resend_retry >= 0 && msg->sms.resend_try >= >> sms_resend_retry) /*check how many times the split message has been sent */ >> + { >> + uuid_unparse(msg->sms.id, id); >> + warning(0, "SMPP[%s] :Maximum retries for message [%s] exceeded >> resend %ld times, discarding it!", octstr_get_cstr(conn->id), id, >> msg->sms.resend_try); >> + handle_split(conn, msg, SMSCCONN_FAILED_DISCARDED); >> + return; >> + } >> + >> + >> + uuid_unparse(msg->sms.id, id); >> + warning(0, "SMPP[%s] :Split message [%s] failed temporary sending it >> to sms_router but with fixed connection", octstr_get_cstr(conn->id), id); >> + >> + msg->sms.resend_try = (msg->sms.resend_try > 0 ? msg->sms.resend_try >> + 1 : 1); /* inc its retries */ >> + debug("bb.sms.splits", 0, "SMPP[%s] :Resend counter of message [%s] >> set to %ld", octstr_get_cstr(conn->id), id, msg->sms.resend_try); >> + time(&msg->sms.resend_time); /* take resend time - this will be used >> in sms_router */ >> + >> + >> + if(uuid_is_null(msg->msg_ConnId) == 1) >> + uuid_copy(msg->msg_ConnId, conn->smscconn_uuid); >> + >> + gwlist_produce(outgoing_sms, msg); /* send it back to global queue >> */ >> + return; >> } >> >> /* >> @@ -1187,8 +1216,8 @@ >> long bp_load, bo_load; >> int i, s, ret, bad_found, full_found; >> long max_queue, queue_length; >> - char *uf; >> - >> + char *uf, id[UUID_STR_LEN + 1]; >> + >> /* XXX handle ack here? */ >> if (msg_type(msg) != sms) { >> error(0, "Attempt to route non SMS message through smsc2_rout!"); >> @@ -1229,6 +1258,29 @@ >> bp_load = bo_load = queue_length = 0; >> >> conn = NULL; >> + /*if msg was a split and failed temporary then use stored connection */ >> + if(uuid_is_null(msg->msg_ConnId) != 1) >> + { >> + for (i=0; i < gwlist_len(smsc_list); i++) >> + { >> + conn = gwlist_get(smsc_list, i); >> + smscconn_info(conn, &info); >> + queue_length += (info.queued > 0 ? info.queued : 0); >> + >> + if(uuid_compare(msg->msg_ConnId, conn->smscconn_uuid) == 0) >> + { >> + if((smscconn_usable(conn,msg) != -1) && (info.status == >> SMSCCONN_ACTIVE && info.queued < max_queue)) >> + { >> + best_preferred = conn; >> + uuid_unparse(msg->sms.id, id); >> + debug("bb.sms",0,"Message [%s] with pre-set connection >> found", id); >> + continue; >> + } >> + } >> + } >> + } >> + else >> + { >> for (i=0; i < gwlist_len(smsc_list); i++) { >> conn = gwlist_get(smsc_list, (i+s) % gwlist_len(smsc_list)); >> >> @@ -1265,6 +1317,8 @@ >> bo_load = info.load; >> } >> } >> + } >> + >> queue_length += gwlist_len(outgoing_sms); >> if (max_outgoing_sms_qlength > 0 && !resend && >> queue_length > gwlist_len(smsc_list) * max_outgoing_sms_qlength) { >> Index: msg.c >> =================================================================== >> --- msg.c (revision 4843) >> +++ msg.c (working copy) >> @@ -98,6 +98,7 @@ >> msg = gw_malloc_trace(sizeof(Msg), file, line, func); >> >> msg->type = type; >> + uuid_clear(msg->msg_ConnId); >> #define INTEGER(name) p->name = MSG_PARAM_UNDEFINED; >> #define OCTSTR(name) p->name = NULL; >> #define UUID(name) uuid_generate(p->name); >> @@ -113,7 +114,7 @@ >> Msg *new; >> >> new = msg_create(msg->type); >> - >> + uuid_copy(new->msg_ConnId, msg->msg_ConnId); >> #define INTEGER(name) p->name = q->name; >> #define OCTSTR(name) \ >> if (q->name == NULL) p->name = NULL; \ >> Index: msg.h >> =================================================================== >> --- msg.h (revision 4843) >> +++ msg.h (working copy) >> @@ -78,7 +78,7 @@ >> >> typedef struct { >> enum msg_type type; >> - >> + uuid_t msg_ConnId; >> #define INTEGER(name) long name; >> #define OCTSTR(name) Octstr *name; >> #define UUID(name) uuid_t name; >> Index: smscconn.c >> =================================================================== >> --- smscconn.c (revision 4843) >> +++ smscconn.c (working copy) >> @@ -157,13 +157,16 @@ >> Octstr *denied_prefix_regex; >> Octstr *preferred_prefix_regex; >> Octstr *tmp; >> - >> + char id[UUID_STR_LEN + 1]; >> + >> if (grp == NULL) >> return NULL; >> >> conn = gw_malloc(sizeof(*conn)); >> memset(conn, 0, sizeof(*conn)); >> >> + uuid_generate(conn->smscconn_uuid); >> + >> conn->why_killed = SMSCCONN_ALIVE; >> conn->status = SMSCCONN_CONNECTING; >> conn->connect_time = -1; >> @@ -295,7 +298,10 @@ >> gw_assert(conn->send_msg != NULL); >> >> bb_smscconn_ready(conn); >> - >> + >> + uuid_unparse(conn->smscconn_uuid, id); >> + debug("smscconn",0,"Adding smsc connection [%s] with id [%s]", >> octstr_get_cstr(conn->id), id); >> + >> return conn; >> } >> >> Index: smscconn_p.h >> =================================================================== >> --- smscconn_p.h (revision 4843) >> +++ smscconn_p.h (working copy) >> @@ -146,6 +146,8 @@ >> #include "smscconn.h" >> >> struct smscconn { >> + /* variable added in order to uniquely identify each smsc connection */ >> + uuid_t smscconn_uuid; >> /* variables set by appropriate SMSCConn driver */ >> smscconn_status_t status; /* see smscconn.h */ >> int load; /* load factor, 0 = no load */ > >
