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
Ben Suffolk schrieb:
Hi Guys,
For load balancing, and resilience, I have 2 servers, each with
un-named, and named smsbox processes. Incoming messages destined for the
un-named boxes get delivered across the un-named boxes on both servers
quite happily.
However messages for a named box only ever get sent to the last box that
connects with that name, (even though both are connected). If that box
disconnects, messages get queued on bearerbox instead of being sent to
the connected box with the same name.
I can see the problem, in bb_boxc the route_incomming_to_boxc function a
named box is retrieved by a dict_get on smsbox_by_id. So this can only
ever look at the most recently connected box of a given name.
Clearly a way I could implement a fix for this would be to loop over
each box in the list and pull out the ones with the correct name. But I
notice a warning about making sure code is efficient in this function.
However I also notice that for an un-named box the entire list is looped
over anyway.
What is the general consensus on the proper way to fix this?
I guess the most efficient method would be to have the dictionary hold a
list of boxes that share the same name, and then only loop over them.
Thus saving the string comparison on each box name.
Regards
Ben
Index: gw/bb_boxc.c
===================================================================
RCS file: /home/cvs/gateway/gw/bb_boxc.c,v
retrieving revision 1.91
diff -a -u -p -r1.91 bb_boxc.c
--- gw/bb_boxc.c 9 Jan 2008 20:06:58 -0000 1.91
+++ gw/bb_boxc.c 28 Jul 2008 21:04:56 -0000
@@ -1523,6 +1523,8 @@ int route_incoming_to_boxc(Msg *msg)
if ((bc != NULL && best != NULL && bc->load < best->load) ||
(bc != NULL && best == NULL)) {
best = bc;
+ /*we are done */
+ break;
}
}
