[PATCH] routing to named SMSBOX issue
Ben Suffolk <[email protected]>
| Newsgroups | gmane.comp.mobile.kannel.devel |
|---|---|
| Message-ID | <[email protected]> |
> Hi Ben, > > I was just too lazy as I implemented this part :) > > The proper fix is to store List in the dict and then pick random box > after lookup. > > Huh and you spotted real bug that we loop over the whole smsbox > list. We should just pick the first that has queue not exceeded. > Something like attached patch. > > Thanks, > Alex Attached is a patch to sort the multiple boxes with the same name routing issue, I also added some code to do as Alex said about picking the first random un-named box thats not full, instead of looping through each box. Thats done basically the same as Alex's single line break addition, but I was able to remove a few unused variables as well. I also corrected a few bits of bad indentation in the routing function whilst I was there. Regards Ben
multi_smsboxid_routing.patch
(application/octet-stream, 11.2 KB)
Index: gw/bb_boxc.c
===================================================================
RCS file: /home/cvs/gateway/gw/bb_boxc.c,v
retrieving revision 1.91
diff -u -r1.91 bb_boxc.c
--- gw/bb_boxc.c 9 Jan 2008 20:06:58 -0000 1.91
+++ gw/bb_boxc.c 29 Jul 2008 13:12:53 -0000
@@ -154,6 +154,7 @@
static int send_msg(Boxc *boxconn, Msg *pmsg);
static void boxc_sent_push(Boxc*, Msg*);
static void boxc_sent_pop(Boxc*, Msg*, Msg**);
+static void boxc_gwlist_destroy(List *list);
/*-------------------------------------------------
@@ -339,19 +340,51 @@
* but we will only consider real identified boxes
*/
if (msg->admin.boxc_id != NULL) {
+
+ /* Only interested if the connection is not named, or its a different name */
+ if(conn->boxc_id == NULL || octstr_compare(conn->boxc_id, msg->admin.boxc_id)) {
+
+ /*
+ * Different name, need to remove it from the old list.
+ *
+ * I Don't think this case should ever arise, but might as well
+ * be safe.
+ */
+ if (conn->boxc_id != NULL) {
+
+ /* Get the list for this box id */
+ List *boxc_id_list = dict_get(smsbox_by_id, conn->boxc_id);
+
+ /* Delete the connection from the list */
+ if(boxc_id_list != NULL) {
+ gwlist_delete_equal(boxc_id_list, conn);
+ }
+
+ octstr_destroy(conn->boxc_id);
+ }
+
+
+ /* Get the list for this box id */
+ List *boxc_id_list = dict_get(smsbox_by_id, msg->admin.boxc_id);
+
+ /* No list yet, so create it */
+ if (boxc_id_list == NULL) {
+ boxc_id_list = gwlist_create();
+ dict_put(smsbox_by_id, msg->admin.boxc_id, boxc_id_list);
+ }
+
+
+ /* Add the connection into the list */
+ gwlist_append(boxc_id_list, conn);
- /* and add the boxc_id into conn for boxc_status() output */
- if (conn->boxc_id != NULL) {
- dict_remove(smsbox_by_id, msg->admin.boxc_id);
- octstr_destroy(conn->boxc_id);
+ conn->boxc_id = msg->admin.boxc_id;
+ }
+ else {
+ octstr_destroy(msg->admin.boxc_id);
}
- conn->boxc_id = msg->admin.boxc_id;
msg->admin.boxc_id = NULL;
- /* add this identified smsbox to the dictionary */
- /* XXX check for equal boxc_id in Dict, otherwise we overwrite it */
- dict_put(smsbox_by_id, conn->boxc_id, conn);
debug("bb.boxc", 0, "boxc_receiver: got boxc_id <%s> from <%s>",
octstr_get_cstr(conn->boxc_id),
octstr_get_cstr(conn->client_ip));
@@ -624,8 +657,15 @@
gw_rwlock_wrlock(smsbox_list_rwlock);
gwlist_delete_equal(smsbox_list, newconn);
if (newconn->boxc_id) {
- dict_remove(smsbox_by_id, newconn->boxc_id);
+
+ /* Get the list, and remove the connection from it */
+ List *boxc_id_list = dict_get(smsbox_by_id, newconn->boxc_id);
+
+ if(boxc_id_list != NULL) {
+ gwlist_delete_equal(boxc_id_list, newconn);
+ }
}
+
gw_rwlock_unlock(smsbox_list_rwlock);
/*
@@ -1172,7 +1212,7 @@
boxid = counter_create();
/* the smsbox routing specific inits */
- smsbox_by_id = dict_create(10, NULL); /* and a hash directory of identified */
+ smsbox_by_id = dict_create(10, (void(*)(void *)) boxc_gwlist_destroy);
smsbox_by_smsc = dict_create(30, (void(*)(void *)) octstr_destroy);
smsbox_by_receiver = dict_create(50, (void(*)(void *)) octstr_destroy);
smsbox_by_smsc_receiver = dict_create(50, (void(*)(void *)) octstr_destroy);
@@ -1414,8 +1454,8 @@
*/
int route_incoming_to_boxc(Msg *msg)
{
- Boxc *bc = NULL, *best = NULL;
- Octstr *s, *r, *rs;
+ Boxc *bc = NULL;
+ Octstr *s, *r, *rs, *boxc_id = NULL;
long len, b, i;
int full_found = 0;
@@ -1424,33 +1464,25 @@
/* msg_dump(msg, 0); */
- /*
- * We have a specific route to pass this msg to smsbox-id
- * Lookup the connection in the dictionary.
- */
+ /* Check we have at least one smsbox connected! */
gw_rwlock_rdlock(smsbox_list_rwlock);
if (gwlist_len(smsbox_list) == 0) {
gw_rwlock_unlock(smsbox_list_rwlock);
- warning(0, "smsbox_list empty!");
+ warning(0, "smsbox_list empty!");
if (max_incoming_sms_qlength < 0 || max_incoming_sms_qlength > gwlist_len(incoming_sms)) {
gwlist_produce(incoming_sms, msg);
- return 0;
- }
- else
- return -1;
+ return 0;
+ }
+ else
+ return -1;
}
+
+ /*
+ * We have a specific route to pass this msg to smsbox-id?
+ */
if (octstr_len(msg->sms.boxc_id) > 0) {
-
- bc = dict_get(smsbox_by_id, msg->sms.boxc_id);
- if (bc == NULL) {
- /*
- * something is wrong, this was the smsbox connection we used
- * for sending, so it seems this smsbox is gone
- */
- warning(0,"Could not route message to smsbox id <%s>, smsbox is gone!",
- octstr_get_cstr(msg->sms.boxc_id));
- }
+ boxc_id = msg->sms.boxc_id;
}
else {
/*
@@ -1465,35 +1497,69 @@
r = (msg->sms.receiver ? dict_get(smsbox_by_receiver, msg->sms.receiver) : NULL);
rs = (os ? dict_get(smsbox_by_smsc_receiver, os) : NULL);
octstr_destroy(os);
- bc = rs ? dict_get(smsbox_by_id, rs) :
- (r ? dict_get(smsbox_by_id, r) : (s ? dict_get(smsbox_by_id, s) : NULL));
+
+ if (rs)
+ boxc_id = rs;
+ else if (r)
+ boxc_id = r;
+ else if (s)
+ boxc_id = s;
}
- /* check if we found our routing */
- if (bc != NULL) {
- if (max_incoming_sms_qlength < 0 || max_incoming_sms_qlength > gwlist_len(bc->incoming)) {
- gwlist_produce(bc->incoming, msg);
- gw_rwlock_unlock(smsbox_list_rwlock);
- return 1; /* we are done */
+
+ /* We have a specific smsbox-id to use */
+ if (boxc_id != NULL) {
+
+ List *boxc_id_list = dict_get(smsbox_by_id, boxc_id);
+ if (boxc_id_list == NULL) {
+ /*
+ * something is wrong, this was the smsbox connection we used
+ * for sending, so it seems this smsbox is gone
+ */
+ warning(0,"Could not route message to smsbox id <%s>, smsbox is gone!",
+ octstr_get_cstr(boxc_id));
}
else {
- gw_rwlock_unlock(smsbox_list_rwlock);
- return -1;
- }
- }
- else if (s != NULL || r != NULL || octstr_len(msg->sms.boxc_id) > 0) {
- gw_rwlock_unlock(smsbox_list_rwlock);
- /*
- * we have routing defined, but no smsbox connected at the moment.
- * put msg into global incoming queue and wait until smsbox with
- * such boxc_id connected.
- */
- if (max_incoming_sms_qlength < 0 || max_incoming_sms_qlength > gwlist_len(incoming_sms)) {
- gwlist_produce(incoming_sms, msg);
- return 0;
+ /* take random smsbox from list, as long as it has space we will use it,
+ * otherwise check the next one.
+ */
+ len = gwlist_len(boxc_id_list);
+ b = gw_rand() % len;
+
+ for(i = 0; i < gwlist_len(boxc_id_list); i++) {
+ bc = gwlist_get(boxc_id_list, (i+b) % len);
+
+ if (bc != NULL && max_incoming_sms_qlength > 0 &&
+ gwlist_len(bc->incoming) > max_incoming_sms_qlength) {
+ bc = NULL;
+ }
+
+ if (bc != NULL) {
+ break;
+ }
+ }
+
+ if (bc != NULL) {
+ bc->load++;
+ gwlist_produce(bc->incoming, msg);
+ gw_rwlock_unlock(smsbox_list_rwlock);
+ return 1; /* we are done */
+ }
+ else {
+ /*
+ * we have routing defined, but no smsbox connected at the moment.
+ * put msg into global incoming queue and wait until smsbox with
+ * such boxc_id connected.
+ */
+ gw_rwlock_unlock(smsbox_list_rwlock);
+ if (max_incoming_sms_qlength < 0 || max_incoming_sms_qlength > gwlist_len(incoming_sms)) {
+ gwlist_produce(incoming_sms, msg);
+ return 0;
+ }
+ else
+ return -1;
+ }
}
- else
- return -1;
}
/*
@@ -1501,15 +1567,14 @@
* a random smsbox.
*/
- /* take random smsbox from list, and then check all smsboxes
- * and select the one with lowest load level - if tied, the first
- * one
+ /* take random smsbox from list, as long as it has space we will use it,
+ * otherwise check the next one.
*/
len = gwlist_len(smsbox_list);
b = gw_rand() % len;
for(i = 0; i < gwlist_len(smsbox_list); i++) {
- bc = gwlist_get(smsbox_list, (i+b) % len);
+ bc = gwlist_get(smsbox_list, (i+b) % len);
if (bc->boxc_id != NULL || bc->routable == 0)
bc = NULL;
@@ -1520,29 +1585,28 @@
bc = NULL;
}
- if ((bc != NULL && best != NULL && bc->load < best->load) ||
- (bc != NULL && best == NULL)) {
- best = bc;
+ if (bc != NULL) {
+ break;
}
}
- if (best != NULL) {
- best->load++;
- gwlist_produce(best->incoming, msg);
+ if (bc != NULL) {
+ bc->load++;
+ gwlist_produce(bc->incoming, msg);
}
gw_rwlock_unlock(smsbox_list_rwlock);
- if (best == NULL && full_found == 0) {
- warning(0, "smsbox_list empty!");
+ if (bc == NULL && full_found == 0) {
+ warning(0, "smsbox_list empty!");
if (max_incoming_sms_qlength < 0 || max_incoming_sms_qlength > gwlist_len(incoming_sms)) {
gwlist_produce(incoming_sms, msg);
- return 0;
+ return 0;
}
else
- return -1;
+ return -1;
}
- else if (best == NULL && full_found == 1)
+ else if (bc == NULL && full_found == 1)
return -1;
return 1;
@@ -1614,3 +1678,15 @@
gwlist_remove_producer(flow_threads);
}
+
+
+/*
+ * Simple wrapper to allow the named smsbox Lists to be
+ * destroyed when the smsbox_by_id Dict is destroyed
+ *
+ */
+static void boxc_gwlist_destroy(List *list)
+{
+ gwlist_destroy(list, NULL);
+}
+