coredump in zeromq code, related to flapping code about circus

[tarekziade]

I think it makes a lot of sense to set a lock there.

[benoitc]

maybe we should rather queue calls there instead of locking with an async scheduler in the ioloop triggering the calls? That will remove the need of locks and would scale better imo.

We indeed need to handle the message very fast there, and using a lock could slow the process too much.

[fetep]

afaict, call() sends a message and grabs it's reply, and functions that use call() look at the reply. so i think we might have to deal with the Lock, because before we can return we have to send and get a reply. i haven't done a lot of python async programming, so I might be missing something obvious, sorry :)

[benoitc]

sory for the spam, i had a cache issue there. Anyway let me know if the change fix the issue for you since I wasn't able to reproduce it with 100 processes around. We may still need a way to block on call anyway but it should already been handled in the recv command since the iostream is internally using a queue.

[tarekziade]

@benoitc I think you should rename cast into "async_call" because the semantics is not super clear. Also, what happens if I do want to get back the async result ? I think we need for that new API an optional callback for this.

I also agree with @fetep that the call() api needs a lock

[benoitc]

well casting is a a synchronous call without waiting an answer. That's diferrent from an async call. If you look the patch I kept the call for when it's needed. Not sure why you would need a callback there. If you want a callback thing you could use an IOStream.

Did you test it without the lock? Basically the IOstream use a queue, so the lock shouldn't be needed so the call should be OK there. Also I would love to have a test to reproduce the problem. I'm afraid that if we need a lock there it's hiding a scalability problem. We should rather make sure nothing locks.

[tarekziade]

what do you mean by 'casting' exactly ? "broadcasting" ?

how is that different from an async call, and how would I get back my result in async call without a callback ?

re test: pete gave one I believe, so it would be a good thing to have it in our test case indeed.

last: what's your problem with locks, I don't understand it

[benoitc]

i mean casting as to throw away... Which is exactly what we do When you don't need to get the result you use this method.

The problem with locking is that you're locking everything in the ioloop. We shouldn't have to do that there. Like I said the messages falling in the handle functions are queued before, so in theory the call should be OKish. I'm not against the lock as a temporary solution anyway but would be good to know if casting send like my patch does isn't enough.

[tarekziade]

if you throw away the result, I am not sure to understand why a queue is needed at all.

I think we're talking about many different things in this ticket now.

I think we need to focus on fixing the bug first, then discuss the various semantics separately (sync vs async vs throw away)

I propose that we convert Pete's example into a test, make sure we can reliably reproduce the bug, then discuss the possible fix. then open another bug about what kind of extra APIs are needed for our messaging needs.

[benoitc]

not sure to follow. I didn't say it was needed.

The patch I submitted should fix that issue. Explanation coming with explained the reasoning. All the rest is an explanation about the pyzmq internals and why we shouldn't lock.

[tarekziade]

let me restate :

1/ until we have a test case that reproduce the bug, we're just making assumption.
2/ if the lock fix is really slowing down thing, let's have a test that benchmark this part of the code to measure its speed.

[tarekziade]

I commited @benoitc fix but I am quite unhappy with the output.

I think we're moving into a position where we don't really know what's going on in the flapping messages. I think we should make this story better