ensure iopub subscriptions propagate prior to accepting websocket connections

[SylvainCorlay]

Nudge kernel with info request upon opening websocket until some IOPub message is received.

• repeat info requests every 500ms until iopub is received
• busy kernels are not nudged, which would block connections to busy kernels indefinitely

cf jupyter/jupyter_client#593

[jasongrout]

I thought that there was one set of zmq channels (shell, iopub, etc.) shared across all websocket connections (but I haven't looked at the code carefully recently). Is that wrong, or is that changing in this PR?

[SylvainCorlay]

I thought that there was one set of zmq channels (shell, iopub, etc.) shared across all websocket connections (but I haven't looked at the code carefully recently). Is that wrong, or is that changing in this PR?

I think that it is a correct statement.

[minrk]

I thought that there was one set of zmq channels

Each websocket client gets its own zmq connection to the kernel, created here using KernelManager.connect_iopub(), etc.

This is what allows the notebook server to not need client-identifying logic, since reply-routing on e.g. stdin, stream channels is handled in the kernel. IOPub messages are indeed wastefully duplicated, however.

[minrk]

Having poked and prodded quite a bit now, I understand how both branches need a nudge in case of restart:

• zmq sockets are not recreated when the kernel is restarted (they shouldn't need to be, as zmq does its own underlying auto reconnect).
• There is no similar nudge operation triggered on restart for 'existing' connections
• To prod connections, clients tend to recreate websocket connection on restart (this is not strictly required, but it is a standard practice and may be needed in some cases where the zmq ports change)
• a 'reconnecting' websocket may not actually mean a new zmq socket (the 'restoring connection' branch is taken), if the same session id is re-used

So it's possible for a 'new' websocket connection, as part of the client's restart steps, to re-use zmq sockets and thus never nudging the new kernel's sockets.

The downside of doing this nudge on every ws connect, even on established, already-nudged zmq sockets, is that websocket connections will not be accepted while the kernel is busy, even though they should be. Currently, it is valid to connect and start sending requests without waiting for iopub, but this can result in the issues folks are seeing. The most robust and efficient nudge would be on:

1. new zmq connections, and
2. kernel restarts for all existing sockets

where requests on existing, open sockets are blocked until the nudge resolves, not just connection-opening. That's more complicated, and I think we can go with this solution as first pass, and refine it later, especially if the delay is bothering folks.

The last thing I want to look at before merging is what happens if nudge is still outstanding when the connections are closed, then I think this is okay to merge. Thanks @SylvainCorlay!

[minrk]

I noticed that there are test failures caused by a change in status messages delivered. The nudge waits for any iopub message, then resolves, which means it will tend to resolve on the 'busy' status message, and the 'idle' status message will end up propagating to the client after resolving the connection. It may be better to wait for specifically the idle message associated with the info request to preserve message expectations, even though this is not strictly required.

[SylvainCorlay]

@minrk I think this is ready to go.

When this gets in I will update the jupyter_server PR.

[Zsailer]

Thanks, @SylvainCorlay, @minrk, and @jasongrout!

[jasongrout]

Just checking - the function argument is f and the loop variable is f. Is that going to cause a problem?

[jasongrout]

@kevin-bates - what is the plan for a release with this PR merged? We're seeing consistent problems that this PR is designed to fix, and would love to have a release fixing the problem. I'm happy to help with the release process if that is helpful.

[Zsailer]

I can work on a patch release, since @kevin-bates is on vacation this month.

[jasongrout]

Awesome, thanks @Zsailer! Again, I'm happy to help if needed.

[Zsailer]

Oops, I thought I was a maintainer on PyPI for this repo, but it looks like I was mistaken. 🤦

[jasongrout]

I thought I was too. Looks like @blink1073 and @minrk are owners of the notebook package on pypi, though - maybe they could make both of us maintainers too?

[blink1073]

I am not an owner, only a maintainer.

[Zsailer]

Notebook 6.1.6 released with this PR included.