[Pvfs2-developers] Re: the halloween bug fixed
slang at mcs.anl.gov
Thu Oct 11 11:57:01 EDT 2007
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.
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?
> By the way, I'm glad you are doing some work on hash tables for
> these things- these linked lists have always bugged me :)
More information about the Pvfs2-developers