Codecov Report

All modified and coverable lines are covered by tests ✅

Please upload report for BASE (branch-3.8@d8e81f2). Learn more about missing BASE report.

@@              Coverage Diff              @@
##             branch-3.8   #14201   +/-   ##
=============================================
  Coverage              ?   93.24%           
=============================================
  Files                 ?      284           
  Lines                 ?    20248           
  Branches              ?        0           
=============================================
  Hits                  ?    18880           
  Misses                ?     1368           
  Partials              ?        0           

Hi @nojaf thanks for your interest in this. I had imagined in the past that "reconnect" would actually mean generating and refreshing an entirely new session on the page, i.e "automatic reloading". For instance, one common reason for a disconnect would be that a session was closed and deleted due to an activity timeout. In that case, with the original session destroyed, there is nothing to "actually reconnect" to. It seems to me that actually re-connecting to the same session could be very difficult for other reasons as well:

• In the case of load balancers, how to guarantee that the connection lands on the same identical instance that is hosting the session?
• How are problems with inconsistent state to be addressed, e.g. if a connection dropped in the middle of an update.

I'm not actually sure how or if "actual reconnection to the same session" can be made to work, or how difficult it would be.

If instead, "re-connection" is interpreted to mean "automatic reload" then I think all these questions are sidestepped. That also handles the session timeout case, what we had in mind primarily, which "actual reconnection" would not be able to handle.

cc @bokeh/dev and @jbednar since this is probably of interest to HV.

I think both interpretations of re-connect would be valuable. Re-connecting to an existing session would likely have to simply treat the server side state as the source of truth (at least initially) because any attempt at merging state sounds like a rabbit hole. So it probably has to re-request the full session state.

The load balancer issue seems like something that can be solved by correctly configuring session stickiness on the load balancer.

So if it can be made to work it would be nice if we try to restore the server session state and if the session has already been cleaned up the server just creates and sends a new session.

Hi folks!

Re-connecting to an existing session would likely have to simply treat the server side state as the source of truth (at least initially)

Yes, this is what I'm aiming for in this initial implementation.

My company hopes to achieve the following behavior in the Panel:

A user is on a webpage, updates some parameters, leaves the tab open to look at something else, and later returns to find that a WebSocket has disconnected. Previously, this disconnection occurred silently, leaving the user to continue updating parameters without realizing that the page was no longer responding, which could lead to incorrect results. We have now added a disconnect notification for this.

Ideally, we want the state to remain exactly as it was when the user paused their activity. Users updating parameters will not affect the state for other users, so I expect this state to be retained on the server (unless it deletes all session state upon disconnection).

Not immediately on disconnect, but there is a periodic cleanup job to purge unused sessions. The parameters are tunable but I don't think you'd want to disable it entirely, since otherwise the server would just leak resources continually.

Hello, everyone! What would be a good next step for me to pursue here?

Hello @mattpap, any pointers for the next steps here?

Can we try to push this along without immediately make it dependent on notification system? I'd suggest @hoxbro looks at this.

Basically I suggest that we add Document events for retries for now and only integrate the notification system at a later point.

Can we try to push this along without immediately make it dependent on notification system?

Sure. Adding appropriate events should be sufficient.

I have added ReconnectEvent, which is triggered when a reconnect is successful.

I have noticed even though, the websocket is reconnected, events are no longer send, an example is the below button which updates the text before the reconnect, but no longer works after the reconnect. @mattpap is looking into this.

from bokeh.plotting import curdoc
from bokeh.models import Div, Button
from bokeh.layouts import column

counter = 0

body = Div()

button = Button(label="Click me to update text", button_type="success")


def update_text():
    global counter
    body.text = f"Clicked {counter} times"
    counter += 1


button.on_click(update_text)

update_text()

curdoc().add_root(column(button, body))


from bokeh.events import ReconnectEvent
curdoc().on_event(ReconnectEvent, lambda event: print("reconnect_event", flush=True))

Hi @mattpap, were you ever able to figure out the reconnect problem?

were you ever able to figure out the reconnect problem?

@nojaf, not yet, I'm investigating.

@philippjfr, @hoxbro, so I was actually mistaken with my earlier assessment. Fortunately the problem ended up being trivial to fix. What happened was, when document was rebuilt with fresh session state after a reconnect, it would be first cleared, which would trigger root removal events and destroy state on the server side. We already have a mechanism to prevent server sync while allowing event distribution within bokehjs, so it's essentially a two line fix.

@nojaf, it's fixed now.

Ah, that makes sense, but there's no way I could have tracked that down.

I finalized this PR. Changes:

• renamed the new event to ClientReconnected to match the naming pattern
• made the first reconnect instantaneous and reduced delay
• added tests and fixed token generation in unit tests

Okay, I'm quite happy with the reconnect logic and am also happy to merge it in the current state. That said, part of what we had discussed originally was that we wouldn't just implement automatic re-connects but also add a final event that's triggered when all re-connects failed. This event would somehow hold a "reconnect" function, which could then be used to pop up a dialog that would allow the user to trigger another re-connect attempt.

@philippjfr, what you are describing is what I intend to do separately from this PR. I will start a new issue for this.