On Oct 11, 2007, at 10:15 AM, Phil Carns wrote:
Sam Lang wrote:
The attached patch is the proposed fix for this problem. When
the tcp method receives a disconnect from a peer, it invokes a
callback (bmi_method_addr_forget_callback) into the bmi control
layer to remove the address reference from the list. Maybe I
should also add a counter and limit on how bit the list can get,
al though that would involve potentially forcing long-lived
connections to reconnect periodically, and all methods would have
to implement BMI_set_info (DROP_ADDR).
With tcp, new connections are registered, even if they are from
the same host/port on the peer, whereas the other methods seem to
only register new host/port endpoints that haven't been seen
before. So its not completely clear to me when the other methods
need to call this callback, if at all. There needs to be a
matching bmi_method_addr_forget_callback for each
bmi_method_addr_reg_callback, but if the method only registers a
single address per client, the list won't keep growing, unless we
ever plan to support millions of clients.
Hi Sam,
Thanks for tracking this down! I've been out of the office for a
few days so I am a little behind on the conversation. I have a
comment on the patch, though.
I think that for the most part, it is calling
bmi_method_addr_forget_callback() on pretty much any tcp network
error, since it is being invoked from the tcp_forget_addr()
function which is a general purpose function.
This is fine on the server side, but I suspect (haven't been able
to confirm yet) that it would cause problems on the client side.
Clients resolve a tcp://servername:3334 address into a
PVFS_BMI_addr_t once and then hang onto it forever. If a network
error occurs, then the state machines do not re-resolve it; they
will just retry communication on the same PVFS_BMI_addr_t, which
will have been invalidated by the forget_callback() function.
Would it be better to only call bmi_method_addr_forget_callback()
on addresses within bmi_tcp that were registered using
bmi_method_addr_reg_callback()? I haven't looked yet, but that may
require an extra flag somewhere to record which addresses this
applies to. That way, the only person invalidating these things on
errors will be servers which have anonymous addresses that need to
be cleaned out. Clients with long lived address resolutions would
not be affected. It also makes a little more sense from an API
point of view if these two functions are companions that are called
in the same scenario.
Hi Phil,
I can add a server flag to the tcp addr struct, and only call
forget_addr in that case, but it seems like a bit of hack. Can we
just toss the ref list in the bmi control layer and force methods to
manage their addresses? The ref list is only being used to map an
opaque address pointer to a particular method. Could we just make
the address pointer not opaque and encode the method in there somehow?
-sam
By the way, I'm glad you are doing some work on hash tables for
these things- these linked lists have always bugged me :)
-Phil
_______________________________________________
Pvfs2-developers mailing list
[email protected]
http://www.beowulf-underground.org/mailman/listinfo/pvfs2-developers
