-
Notifications
You must be signed in to change notification settings - Fork 65
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Replace pub with xpub proposal #65
Conversation
Thanks @JohanMabille I think this will be a significant improvement! |
I have no objections. Do we know of any case where a kernel is running without a client, like embedded where you might connect/disconnect every now and then to monitor? Would changing to XPUB block the application until there is a connection ? |
I'm not aware of such a use case.
It won't. XPUB is really similar to PUB, the only difference is that it can detect new subscription / unsubscription and notify the kernel. If you publish message on XPUB while no subscriber has connected, the message is simply discarded. |
Ping @minrk @blink1073 @ellisonbg @fperez @Carreau! We would love to have some feedback on this, as it would greatly simplify the handling of kernels. |
I think this is a great idea. Since PUB is really a special case of XPUB where subscription messages are handled internally, the only backward-incompatible part of this proposal happens when clients start assuming a welcome message will come. Everything else will behave the same with no changes on the kernel or client. Clients not aware of the new message will just get an unrecognized iopub message they can safely ignore. That means that clients can still use a kernel_info request to probe the protocol version to determine if they should expect a welcome message or have to fallback on the current probe & wait loop. There's no downside to kernels migrating promptly, and clients can be conservative about what they do wrt backward-compatibility. I would recommend, for client that want to wait for subscriptions to be ready:
|
This proposal looks great to me, pending incorporation of @minrk's feedback. |
It is not likely I will have time to look at this anytime soon, but I trust
the input of others here and am fine with it moving forward.
…On Thu, Feb 25, 2021 at 4:08 AM Min RK ***@***.***> wrote:
***@***.**** approved this pull request.
------------------------------
In jupyter-xpub/jupyter-xpub.md
<#65 (comment)>
:
> +
+## Problem
+
+A Jupyter kernel uses a PUB socket as a broadcast channel where it publishes various messages (incoming requests, outputs, events). Any client that wants to be notified of what happens in the kernel can simply subscribe to this channel via a SUB socket. The issue with this simple PUB - SUB pattern is that the kernel has no mean to detect client subscriptions, and it will broadcast messages whether a client has subscribed or not.
+
+This is particularly problematic when a client needs to send a bunch of execution requests just after starting a kernel (a typical use case is the "Restart Kernel and execute all" command of Jupyter Lab, or opening a Notebook with Voilà). In that case, the client needs to ensure that its SUB socket is connected to the PUB socket of the kernel before sending any execution request, otherwise it may miss some outputs. The kernel, on its side, will not emit any event until it receives a request.
+
+## Current solution
+
+The current solution consists of sending `kernel_info` requests until a `kernel_info` reply is received on the SHELL and an idle status message is received on the SUB socket. If the client receives a `kernel_info` reply but not the idle status message before a timeout, this means that the SUB socket has not connected yet. The client discards the reply and send a new `kernel_info` request.
+
+This solution makes the implementation of a Jupyter client more complicated than required. Indeed, ZeroMQ provides a publisher socket that is able to detect new subscriptions.
+
+## Proposed Enhancement
+
+We propose to replace the PUB socket of the kernel with an XPUB socket. When it detects a new subscriber, the kernel sends a welcome message to the client. This way, the connecting sequence of the client is simplified: it only has to wait for a `kernel_info` reply and a welcome message on the SUB socket.
I think we should be a little more specific here and specify some example
messages.
XPUB sockets receive the following events:
- subscribe (receives messages with \1{subscription})
- unsubscribe (receives messages with \0{subscription})
Should we send a "farewell" message on unsubscribe? I'm assuming not,
since we don't really know who is unsubscribing.
For example:
When the IOPub XPUB socket receives a message indicating a new
subscription (a single-frame message starting with the byte \x01 followed
by the subscription, it shall send an iopub_welcome message with no
parent header and the received subscription in the subscription field, *with
the given subscription prefix*:
identity_prefix = ["subscription-prefix"]
parent_header = {}
msg_type = "iopub_welcome"
content = {
"subscription": "subscription-prefix",
}
A Python example implementation:
SUBSCRIBE = b"\1"
UNSUBSCRIBE = b"\0"
def notify_subscription(stream, msg_frames):
"""Handle a subscription notification on an XPUB socket"""
msg = msg_frames[0]
op, prefix_bytes = msg[:1], msg[1:]
try:
prefix = prefix_bytes.decode("utf8")
except UnicodeError:
log.warning(f"Received invalid subscription prefix: {prefix_bytes}")
return
if op == SUBSCRIBE:
log.info(f"Received subscription on '{prefix}'")
session.send(
stream,
"iopub_welcome",
content={"subscription": prefix},
ident=[prefix_bytes],
)
elif op == UNSUBSCRIBE:
log.debug(f"Received unsubscribe on '{prefix}'")
else:
log.warning(f"Received unrecognized XPUB event: '{msg}'")
xpub.XPUB_VERBOSE = 1 # ensure we notify on *every* subscription, even duplicates
stream = ZMQStream(xpub)
stream.on_recv_stream(notify_subscription)
(full example with a subscriber
<https://gist.github.com/d59add05d11a40c6f70fd6ed82ada136>)
Notes:
- The XPUB_VERBOSE socket option should be set to 1, such that
repeated subscriptions on a socket still produce notifications
- Subscriptions can be empty (indeed, this is almost always the case).
An empty subscription means subscribing to all iopub messages because zmq
does simple bytestring prefix matching.
- Subscriptions shall be UTF-8-encoded. Non-utf8 subscriptions shall
be ignored, and not produce a welcome message.
- every subscribed client will receive every *other* client's welcome
message, assuming they match existing subscriptions (e.g. empty strings).
- welcome messages do not and cannot identify the client whose
subscription is being received. Receipt of an iopub_welcome message
with your subscription does *not* mean it is in response to your own
subscription. However, receiving a message does mean that *a* matching
subscription has been registered for your client, otherwise no message will
be received. So if only one subscription is registered, as is normally the
case, receiving *any* welcome message is sufficient to indicate that
your client's subscription is fully established. The gist is that receiving
a welcome message is a sufficient condition to establish the
subscription-propagation event, and additional welcome messages should be
expected and ignored.
------------------------------
In jupyter-xpub/jupyter-xpub.md
<#65 (comment)>
:
> +### GitHub repositories
+
+- Jupyter Client: https://github.com/jupyter/jupyter_client
+The Jupyter protocol client APIs
+- Jupyter Server: https://github.com/jupyter-server/jupyter_server
+The backend to Jupyter web applications
+- Jupyter Notebook: https://github.com/jupyter/notebook
+The Jupyter interactive notebook
+- IPyKernel: https://github.com/ipython/ipykernel
+IPython kernel for Jupyter
+- Xeus: https://github.com/jupyter-xeus/xeus
+The C++ implementation of the Jupyter kernel protocol
+
+### GitHub Issues
+
+- Main issue in jpuyter_client: SUB sockets take time to subscribe to the IOPub channel and miss important messages (jupyter/jupyter_client#593)
⬇️ Suggested change
-- Main issue in jpuyter_client: SUB sockets take time to subscribe to the IOPub channel and miss important messages (jupyter/jupyter_client#593)
+- Main issue in jupyter_client: SUB sockets take time to subscribe to the IOPub channel and miss important messages (jupyter/jupyter_client#593)
------------------------------
In jupyter-xpub/jupyter-xpub.md
<#65 (comment)>
:
> @@ -0,0 +1,54 @@
+# Replace PUB socket with XPUB socket
+
+## Problem
+
+A Jupyter kernel uses a PUB socket as a broadcast channel where it publishes various messages (incoming requests, outputs, events). Any client that wants to be notified of what happens in the kernel can simply subscribe to this channel via a SUB socket. The issue with this simple PUB - SUB pattern is that the kernel has no mean to detect client subscriptions, and it will broadcast messages whether a client has subscribed or not.
I would be a little more specific here and say that the issue is entirely
in the client. Maybe start with stating the client issue: *there is no
simple mechanism for clients to wait for their iopub subscriptions to be
established*. The given examples show when clients want to wait for that
condition.
It's not an issue that the kernel doesn't know whether it has any pub/sub
subscribers - this is handled just fine, and it's no problem for there to
be zero subscribers from the kernel's perspective - if there are no
subscribers when it sends an iopub message, discarding those is absolutely
the right thing to do, and not a bug.
The proposed solution also does not require the kernel to 'detect
subscriptions' in that it needs to keep track of whether there are any
currently subscribed clients. This is good! I think it's more clearly a
kernel-side solution to a client-side problem that we can send an 'ack'
message on subscription so that clients can see their own subscriptions
take effect immediately within a single channel. The client-side handling
can still be purely stateless, and unsubscribe events can be ignored.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#65 (review)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAAGXUDCBUCLTEYLKUCU4LLTAY4VVANCNFSM4VUSN5SA>
.
--
Brian E. Granger
Principal Technical Program Manager, AWS AI Platform ([email protected])
On Leave - Professor of Physics and Data Science, Cal Poly
@ellisonbg on GitHub
|
Notes: | ||
|
||
- The identity prefix may contain extra information about the message, such as the kernel id, according to the convention used in the IPython kernel: | ||
```kernel.{u-u-i-d}.subscription-topic```. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
At some point, we should probably consider adopting a spec for IOPub message topic prefixes. I don't think anybody really cares and just subscribes to all, but this is still an unspecified part of the protocol other than this one new message.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think @minrk covered potential feedback I'd of had better than I would have done and I generally agree with his assessments and recommendations here.
Is there any blocker preventing this JEP from being merged? |
Sorry for the lack of attention @JohanMabille! @jupyter/steeringcouncil If there are any aspects of this JEP that need to be reconsidered, please take a look and comment. |
The coming week is the final work week of 2021. This JEP was opened in January of this year. I propose we merge this JEP on Friday 24 December or earlier. If there are any showstoppers, please comment before then. Thanks, everyone! Thank you and apologies to Johan for your patience and perseverance. |
Closes #569 This PR fixes a race condition regarding subscriptions to IOPub that causes clients to miss IOPub messages: - On startup a client connects to the server sockets of a kernel. - The client sends a request on Shell. - The kernel starts processing the request and emits busy on IOPub. If the client hasn't been able to fully subscribe to IOPub, messages can be lost, in particular the Busy message that encloses the request output. On the Positron side we fixed it by sending kernel-info requests in a loop until we get a Ready message on IOPub. This signals Positron that the kernel is fully connected and in the Ready state: posit-dev/positron#2207. We haven't implemented a similar fix in our dummy clients for integration tests and we believe this is what is causing the race condition described in #569. As noted in posit-dev/positron#2207, there is an accepted JEP proposal (JEP 65) that aims at solving this problem by switching to XPUB. https://jupyter.org/enhancement-proposals/65-jupyter-xpub/jupyter-xpub.html jupyter/enhancement-proposals#65 The XPUB socket allows the server to get notified of all new subscriptions. A message of type `iopub_welcome` is sent to all connected clients. They should generally ignore it but clients that have just started up can use it as a cue that IOPub is correctly connected and that they won't miss any output from that point on. Approach: The subscription notification comes in as a message on the IOPub socket. This is problematic because the IOPub thread now needs to listens to its crossbeam channel and to the 0MQ socket at the same time, which isn't possible without resorting to timeout polling. So we use the same approach and infrastructure that we implemented in #58 for listeing to both input replies on the StdIn socket and interrupt notifications on a crossbeam channel. The forwarding thread now owns the IOPub socket and listens for subscription notifications and fowrards IOPub messages coming from the kernel components. --- * Start moving IOPub messages to forwarding thread * Remove unused import * Resolve the roundabout `Message` problem The solution was to move the conversion to `JupyterMessage<T>` up into the match, so we "know" what `T` is! * Use correct `Welcome` `MessageType` * Implement `SubscriptionMessage` support and switch to `XPUB` * The `Welcome` message doesn't come from ark * Use `amalthea::Result` * Add more comments --------- Co-authored-by: Davis Vaughan <[email protected]> Co-authored-by: Lionel Henry <[email protected]>
Closes #569 This PR fixes a race condition regarding subscriptions to IOPub that causes clients to miss IOPub messages: - On startup a client connects to the server sockets of a kernel. - The client sends a request on Shell. - The kernel starts processing the request and emits busy on IOPub. If the client hasn't been able to fully subscribe to IOPub, messages can be lost, in particular the Busy message that encloses the request output. On the Positron side we fixed it by sending kernel-info requests in a loop until we get a Ready message on IOPub. This signals Positron that the kernel is fully connected and in the Ready state: posit-dev/positron#2207. We haven't implemented a similar fix in our dummy clients for integration tests and we believe this is what is causing the race condition described in #569. As noted in posit-dev/positron#2207, there is an accepted JEP proposal (JEP 65) that aims at solving this problem by switching to XPUB. https://jupyter.org/enhancement-proposals/65-jupyter-xpub/jupyter-xpub.html jupyter/enhancement-proposals#65 The XPUB socket allows the server to get notified of all new subscriptions. A message of type `iopub_welcome` is sent to all connected clients. They should generally ignore it but clients that have just started up can use it as a cue that IOPub is correctly connected and that they won't miss any output from that point on. Approach: The subscription notification comes in as a message on the IOPub socket. This is problematic because the IOPub thread now needs to listens to its crossbeam channel and to the 0MQ socket at the same time, which isn't possible without resorting to timeout polling. So we use the same approach and infrastructure that we implemented in #58 for listeing to both input replies on the StdIn socket and interrupt notifications on a crossbeam channel. The forwarding thread now owns the IOPub socket and listens for subscription notifications and fowrards IOPub messages coming from the kernel components. --- * Start moving IOPub messages to forwarding thread * Remove unused import * Resolve the roundabout `Message` problem The solution was to move the conversion to `JupyterMessage<T>` up into the match, so we "know" what `T` is! * Use correct `Welcome` `MessageType` * Implement `SubscriptionMessage` support and switch to `XPUB` * The `Welcome` message doesn't come from ark * Use `amalthea::Result` * Add more comments --------- Co-authored-by: Davis Vaughan <[email protected]> Co-authored-by: Lionel Henry <[email protected]>
Replace PUB socket with XPUB socket
Problem
A Jupyter kernel uses a PUB socket as a broadcast channel where it publishes various messages (incoming requests, outputs, events). Any client that wants to be notified of what happens in the kernel can simply subscribe to this channel via a SUB socket. The issue with this simple PUB - SUB pattern is that there is no simple mechanism for clients to wait for their iopub subscription to be established.
This is particularly problematic when a client needs to send a bunch of execution requests just after starting a kernel (a typical use case is the "Restart Kernel and execute all" command of Jupyter Lab, or opening a Notebook with Voilà). In that case, the client needs to ensure that its SUB socket is connected to the PUB socket of the kernel before sending any execution request, otherwise it may miss some outputs. The kernel, on its side, will not emit any event until it receives a request.
Current solution
The current solution consists of sending
kernel_info
requests until akernel_info
reply is received on the SHELL and an idle status message is received on the SUB socket. If the client receives akernel_info
reply but not the idle status message before a timeout, this means that the SUB socket has not connected yet. The client discards the reply and send a newkernel_info
request.This solution makes the implementation of a Jupyter client more complicated than required. Indeed, ZeroMQ provides a publisher socket that is able to detect new subscriptions.
Proposed Enhancement
We propose to replace the PUB socket of the kernel with an XPUB socket. XPUB sockets receive the following events:
\1{subscription-topic}
)\0{subscription-topic}
)When the IOPub XPUB socket receives an event indicating a new subscription, it shall send an
iopub_welcome
message with no parent header and the receivedsubscription in the
subscription
field, with the given subscription topic:Notes:
kernel.{u-u-i-d}.subscription-topic
.Impact on existing implementations
Although this enhancement requires changing all the existing kernels, the impact should be limited. Indeed, most of the kernels are based on the kernel wrapper approach, or on xeus.
Regarding clients, this change is backward-incompatible only when they start assuming a welcome message will come. Clients not aware of
the new message will just get an unrecognized iopub message they can safely ignore.
Most of the clients are based on the Jupyter Server. Therefore, the changes should be limited to this repository and to the notebook. The new
recommended starting procedure is:
kernel_info_request
(required for protocol adaptation)i. if welcome message is supported, wait for it on iopub
ii. if not:
a. use current request & wait loop (recommended, especially for current clients that already have this implemented)
b. accept and warn about possible loss of early iopub messages for 'old' kernels ((increasingly common over time as protocol updates can be more safely assumed, especially for new clients
Relevant Resources (GitHub repositories, Issues, PRs)
GitHub repositories
The Jupyter protocol client APIs
The backend to Jupyter web applications
The Jupyter interactive notebook
IPython kernel for Jupyter
The C++ implementation of the Jupyter kernel protocol
GitHub Issues
GitHub Pull Requests
cc @SylvainCorlay @jtpio @martinRenou @maartenbreddels @minrk
EDIT: incorporated feedbacks from @minrk