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@beowulf-underground.org
http://www.beowulf-underground.org/mailman/listinfo/pvfs2-developers
_______________________________________________
Pvfs2-developers mailing list
Pvfs2-developers@beowulf-underground.org
http://www.beowulf-underground.org/mailman/listinfo/pvfs2-developers
