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.
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
