These are chat archives for ipython/ipython

23rd
Feb 2015
Jason Grout
@jasongrout
Feb 23 2015 20:41 UTC
@jdfreder, @minrk - there's a subtle race condition in our message processing
message processing for comms
Sylvain Corlay
@SylvainCorlay
Feb 23 2015 20:42 UTC
:horse:
Jason Grout
@jasongrout
Feb 23 2015 20:43 UTC
basically, the kernel (after ipython/ipython#7780) correctly queues up comm messages into each individual queue in order
but the queues themselves can execute async from each other.
here's a situation that can happen:
box = Box()
display(box)
a = IntSlider()
box.children = [a]
the create comm message is sent for box and the slider.
sorry, the messages are sent in this order:
box create -> creates comm_box message queue
box update -> queued up in the comm_box message queue to update the box model
a create -> creates comm_a message queue
a update -> queued up in the comm_a message queue to update the a model
box update -> queued up in comm_box message queue to update the box model (which triggers a creation of the a slider view)
Jonathan Frederic
@jdfreder
Feb 23 2015 20:48 UTC
Another race condition :worried:
Jason Grout
@jasongrout
Feb 23 2015 20:49 UTC
so then, the message queues look like this:
box queue: create, update, update2
a queue: create, update
Jonathan Frederic
@jdfreder
Feb 23 2015 20:49 UTC
I see
hmm
Jason Grout
@jasongrout
Feb 23 2015 20:49 UTC
problem happens when a's update happens after box's update2. The box tries to create a view of a model that hasn't been initialized
Jonathan Frederic
@jdfreder
Feb 23 2015 20:49 UTC
So the async promise sync stuff needs to happen lower?
Jason Grout
@jasongrout
Feb 23 2015 20:49 UTC
but more generally, it's surprising that comm messages are executed in a different order than they are queued, if they happen on two different comm message queues
so we have a number of solutions (we had a short discussion with a bunch of us)
Jonathan Frederic
@jdfreder
Feb 23 2015 20:50 UTC
We definitely do not want to implement promises at the websocket
atleast, not for 3.x
maybe something that sits in between
Jason Grout
@jasongrout
Feb 23 2015 20:50 UTC
well, that's basically what ipython/ipython#7780 does for serialization
Jonathan Frederic
@jdfreder
Feb 23 2015 20:51 UTC
a singleton method for comms
Jason Grout
@jasongrout
Feb 23 2015 20:51 UTC
but no, the issue is the order the comm messages are processed
so we can make a single overarching comm message queue, instead of separate queues for each comm
Jonathan Frederic
@jdfreder
Feb 23 2015 20:52 UTC
yes, a singleton method
Does ipython/ipython#7780 fix this problem?
Jason Grout
@jasongrout
Feb 23 2015 20:52 UTC
there are also a number of ways to address this specific issue in widgets, but that is just hiding the async confusion
no, #7780 fixes a separate problem in deserializing messages
Jonathan Frederic
@jdfreder
Feb 23 2015 20:53 UTC
Well it seems like the async rabbit hole just gets deeper and deeper.
Jason Grout
@jasongrout
Feb 23 2015 20:53 UTC
async :rabbit:
Jonathan Frederic
@jdfreder
Feb 23 2015 20:54 UTC
What are the alternatives to the single message queue idea?
Jason Grout
@jasongrout
Feb 23 2015 20:54 UTC
well, that's the root of the issue
Jonathan Frederic
@jdfreder
Feb 23 2015 20:54 UTC
Ok
That's probably what we need to do then
Do you want to open a PR ?
otherwise I can-
Jason Grout
@jasongrout
Feb 23 2015 20:54 UTC
an alternative is to make things async, but make a specific context handler or something execute comm messages sync.
but that's just exposing the async confusion to the user.
I'd rather default to sync, and then make a special context handler for async messages
that can then use Promise.all() in js to process a collection of messages, or something
But I'm also curious what @minrk's thoughts are. He's dealt with async stuff too.
I hate to make things sync when most of the time it won't matter, but sometimes comm messages presume previous messages to other queues have executed
Sylvain Corlay
@SylvainCorlay
Feb 23 2015 20:57 UTC
we had a some ideas on how to make things being processes synchronously and potentially asynchronously when needed.
from the python side
Jason Grout
@jasongrout
Feb 23 2015 20:58 UTC
another way to expose the async to the user is to make them be explicit about when there are dependencies between comms. comm.send(..., depends_on_comms=[commA, commB, commC]). Widgets, when they realize they are sending a model reference, would expose that dependency on the other widget model's comms, i.e., if a widget sent an update where they are sending a model reference, they could explicitly depend on that other widget's comm.
Sylvain Corlay
@SylvainCorlay
Feb 23 2015 20:58 UTC

like

with All(w1, w2, w3):
    w1.foo = ...
    w2.bar = ...

to mean that updates of w1 and w2 may be processes asynchronously

Jonathan Frederic
@jdfreder
Feb 23 2015 20:59 UTC
mmm :/ both don't feel right
I think I like the single queue
I think @minrk is away
Sylvain Corlay
@SylvainCorlay
Feb 23 2015 21:00 UTC
ie, the widget manager has a single processing queue, making processing synchronous, except when spcified otherwise
Jason Grout
@jasongrout
Feb 23 2015 21:00 UTC
the easiest solution is to just make comm messages synchronous.
Sylvain Corlay
@SylvainCorlay
Feb 23 2015 21:01 UTC
yes, the All idea is to allow them to be locally async when you know they can be
Jonathan Frederic
@jdfreder
Feb 23 2015 21:03 UTC
I don't think exposing that to the user makes sense
sounds like a WTF moment
I think the single queue idea is still the best we have
Sylvain Corlay
@SylvainCorlay
Feb 23 2015 21:05 UTC
@jdfreder I think that we agree.. The All stuff is to locally allow things to be async. ie, when I don't care about the order for w1, w2, w3
Sylvain Corlay
@SylvainCorlay
Feb 23 2015 21:21 UTC
do you see what I mean?
implementationwise, All would have it own queue taking all messages originally meant for w1 w2 w3 and the js implementation of the All widget would process the messages without the synchronicity constraints to which the main widget manager is bound
Jonathan Frederic
@jdfreder
Feb 23 2015 21:23 UTC
yeah
I understand
Sylvain Corlay
@SylvainCorlay
Feb 23 2015 21:24 UTC
a bit like with hold_sync... It is an advance feature of the API
Jason Grout
@jasongrout
Feb 23 2015 21:33 UTC
@jdfreder - I'll work on a patch
Jason Grout
@jasongrout
Feb 23 2015 21:47 UTC
@jdfreder - we could go one step further and insist that all kernel messages that return promises are always resolved before moving to the next kernel message
that really hits the messages with the sync hammer
Jonathan Frederic
@jdfreder
Feb 23 2015 21:48 UTC
Wouldn't that cause the promise logic to propagate further than necessary?
i.e. into other kernel message handlers
Jason Grout
@jasongrout
Feb 23 2015 21:49 UTC
"necessary" -- that's an interesting word :)
it would ensure that every message is completely handled in the order it appears
and the promise logic is already in the kernel message handler, courtesy of #7780
Sylvain Corlay
@SylvainCorlay
Feb 23 2015 22:15 UTC
btw @jdfreder do you plan on opening a PR with your work on dynamic traitlets?
Jonathan Frederic
@jdfreder
Feb 23 2015 22:15 UTC
The student is going to open one with his solution, which doesn't use metaclasses.
I'll open mine as an alternative
Sylvain Corlay
@SylvainCorlay
Feb 23 2015 22:15 UTC
ok
Jonathan Frederic
@jdfreder
Feb 23 2015 22:16 UTC
But I think his method may be the way to go.
Sylvain Corlay
@SylvainCorlay
Feb 23 2015 22:16 UTC
yours is really short
Jonathan Frederic
@jdfreder
Feb 23 2015 22:16 UTC
Mines pretty hacky, reassigning __class__ and all...
Yeah
his solution is ~50 lines or so
Sylvain Corlay
@SylvainCorlay
Feb 23 2015 22:17 UTC
I played with it a little bit and it works nicely
Jonathan Frederic
@jdfreder
Feb 23 2015 22:17 UTC
and requires __getattr__ and __setattr__ to be overridden.
Thanks
Sylvain Corlay
@SylvainCorlay
Feb 23 2015 22:18 UTC
SylvainCorlay/ipython@24871d4
Jonathan Frederic
@jdfreder
Feb 23 2015 22:19 UTC
Cool :D
Sylvain Corlay
@SylvainCorlay
Feb 23 2015 22:20 UTC
Oh, btw, regarding traitlets, besides the few already open PRs, we started discussing some deeper changes here
which could not be split into super small PRs
I would love to discuss this beforehand, and have a design discussion with you guys to know what you are open to, rather than doing a lot of work and not get in.
This message was deleted
Jonathan Frederic
@jdfreder
Feb 23 2015 22:22 UTC
@SylvainCorlay I'd love to do that too
Who else do you think should be in on the call?
I'm probably the most flexible in time.
Sylvain Corlay
@SylvainCorlay
Feb 23 2015 22:23 UTC
I am quite flexible as well, but today will probably not work for me.
I guess having Jason and Min. The goal would be to start discussions on the support of containers
this is very linked to the discussions we had on the typing
Jonathan Frederic
@jdfreder
Feb 23 2015 22:38 UTC
ipython/ipython#7837 :)
Jason Grout
@jasongrout
Feb 23 2015 23:15 UTC
@jdfreder: async :rabbit2:s at ipython/ipython#7838 and ipython/ipython#7839 (pick one)