[Pvfs2-developers] bmi testcontext/testunexpected

[walt]

I'm not sure this is exactly the issue.  Having a single call to get 
expected or unexpected or both is a good idea, but the issue here seems 
to be the call waiting until the desired context has completed ops 
versus returning short because there are operations from other contexts 
(or unexpecteds) that do have completions.

Sounds like the ideal API would look something like this:

bmi_testcontext(<set of contexts we want to get>,
                 <set of contexts we should return if are available>)

then we can get the original behavior by setting the second set to the
mythical context of unexpected messages, and the desired behavior for 
zoid by using an empty set for the second set.

With this the application decides what the "priorities" or "schedule" is 
by how it builds these sets.

:D

Walt

Phil Carns wrote:
> Yeah, I don't particularly like adding special cases either.
> 
> I feel like making the consumer play with timeouts or use an extra 
> thread would be just as much of a hack/workaround, though.  Its just 
> moving the problem elsewhere.
> 
> Fundamentally it seems more like a BMI API flaw.  It would have made 
> more sense (for example) if unexpected messages were assigned to a 
> specific context and the testunexpected() and testcontext() functions 
> were combined.  The consumer could then use a single test call to 
> retrieve both unexpected and normal messages at once if they are in the 
> same context (as in the pvfs2-server use case).  Testing on a different 
> context would ignore the presence of unexpected messages (as in the 
> problem triggering use case here).
> 
> There are other ways to deal with it, that's just an example.  We just 
> need the API to better express the intention of the caller (preferably 
> in one function) so that BMI doesn't have to optimize by guessing about 
> what else is going on.
> 
> That is more work than just adding a flag, though :)  It probably 
> depends on if we think the use case is going to be around long enough to 
> justify tweaking the API.
> 
> -Phil
> 
> Sam Lang wrote:
>>
>> I've committed the set_info fix for this.  I'm not crazy about it, but 
>> it should work for now.  In the long term, we should probably move 
>> away from method specific hacks like this.  I.e. it should be up to 
>> the API consumer (our server) to adjust timeouts or call 
>> testunexpected in a separate thread.
>>
>> Nawab, in the zoidfs init code after initializing BMI you need to call:
>>
>> int check = 0;
>> BMI_set_info(0, BMI_TCP_CHECK_UNEXPECTED, &check);
>>
>> -sam
>>
>> On Dec 23, 2008, at 2:01 PM, Phil Carns wrote:
>>
>>> Sam Lang wrote:
>>>> Hi All,
>>>> I think Nawab has found a bug (or untested code path) in the BMI tcp 
>>>> method.  He's running a daemon that both receives unexpected 
>>>> requests (as a server), and receives expected responses (as a client).
>>>> In the BMI_testcontext call, if there aren't any completed 
>>>> (expected) operations, and there are completed unexpected receives, 
>>>> we return immediately, assuming that BMI_testunexpected will be 
>>>> called in turn.  I think the idea here is that we want to keep our 
>>>> latency down for unexpected messages, instead of doing work on 
>>>> expected messages while unexpected messages are waiting in the 
>>>> hopper.  But the daemon is single threaded, and making blocking 
>>>> PVFS_sys_* calls, so we essentially spin forever calling 
>>>> BMI_testcontext over and over.
>>>> I'm not sure of the best way to fix this.  Easy fixes would be to 
>>>> remove the check for completed unexpected receives, and/or do 
>>>> tcp_do_work for a shorter timeout.
>>>> It seems like we have a special case for blocking PVFS_sys_* calls.  
>>>> We want to ignore unexpected receives just in that case, and 
>>>> actually call tcp_do_work.  In other contexts, I think we want the 
>>>> behavior that we have now, where we assume that a BMI_testunexpected 
>>>> call will follow a BMI_testcontext call.  We could modify the 
>>>> testcontext call to take a separate parameter, but that seems 
>>>> messy.  We might also be able to handle this with separate BMI 
>>>> contexts somehow...
>>>
>>> I haven't dug in the code yet to see if I see any more elegant way to 
>>> handle it, but I wanted to mention that if you want to add a special 
>>> flag to toggle the behavior, it might be better to just set it 
>>> globally with the set_info() function rather than modifying the 
>>> testcontext() api.  That way you don't have to change any of the 
>>> other BMI methods. There are already a couple of similar set_info() 
>>> calls to toggle BMI behavior for different use cases.
>>>
>>> -Phil
>>
> 
> _______________________________________________
> Pvfs2-developers mailing list
> Pvfs2-developers at beowulf-underground.org
> http://www.beowulf-underground.org/mailman/listinfo/pvfs2-developers