Skip to content
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

Implement strategy to handle idle connections #116

Open
6 tasks
ecton opened this issue Dec 10, 2021 · 3 comments
Open
6 tasks

Implement strategy to handle idle connections #116

ecton opened this issue Dec 10, 2021 · 3 comments
Labels
enhancement New feature or request networking Issues relating to either the networked server or client
Milestone

Comments

@ecton
Copy link
Member

ecton commented Dec 10, 2021

Original issue is in comment 2.

@daxpedda introduced max_idle_timeout in fabruic, which offers a simple solution to the problem, but after thinking about it, we need more logic in BonsaiDb itself to handle this in ways that I would want as a database administrator.

The Fabruic setting can only be done at initialization time, and it doesn't impact WebSocket clients. To me, BonsaiDb's idle strategy needs to work similarly for all types of connections. Additionally, if we can't turn on or off the idle timeout, then features like PubSub start requiring reconnection to function. Finally, authentication requires a back-and-forth negotiation, and we shouldn't force clients to reconnect and reauthorize unless absolutely necessary.

Thus, here's what I think we should do:

  • Add new Server configuration options:
    • idle timeout
    • authenticated idle timeout
    • server keepalive interval (must be less than idle timeout)
  • Set Fabruic's the max_idle_timeout to some reasonable value within BonsaiDb. quinn's documentation warns against using None due to malfunctions or bad actors being able to cause futures that never time out.
  • Introduce KeepAlive on both Request and Response, perhaps with unique names to indicate the direction of flow.
  • Introduce a server-generated KeepAlive:
    • If the connection should be kept alive (authenticated, or there's an active PubSub subscriber), the server should send a KeepAlive response based on the keepalive interval configuration
    • The client should respond with a KeepAlive request whenever it receives the server message.
  • Update the server to keep track of the last time a Request was seen.
  • Update the loops that fetch new requests from a client to have timeouts based on the last time a request was seen and the correct idle timeout based on the connection's authenticated state.
@daxpedda
Copy link
Member

This might be a QUIC timeout thing, tell me if it's that, then I can expose a setting that disables it or let's you set whatever you like. If you like to solve it from your side you can send a ping or something to prevent it from timeout.

@ecton
Copy link
Member Author

ecton commented Dec 10, 2021

I appreciate your quick reply! I was recording the creation of khonsulabs/minority-game and realized that the QUIC connections were disconnecting after no traffic was sent. I'll take a look at what's happening and let you know what I think we should do.

Most of the new issues from today/yesterday that have very little info were just created on-the-fly as I was recording so that I didn't forget.

@ecton ecton changed the title Investigate bonsaidb connections dropping after ~10 seconds Implement strategy to handle idle connections Dec 12, 2021
ecton added a commit that referenced this issue Dec 12, 2021
This makes the QUIC connections behave similarly to the WebSocket
connections by not timing out automatically. However, this is a band-aid
and a full solution is being suggested in #116
@ecton ecton added this to the v0.1.0 milestone Dec 15, 2021
@ecton ecton added the server label Dec 15, 2021
@ecton ecton added enhancement New feature or request networking Issues relating to either the networked server or client and removed server labels Jan 26, 2022
@ecton ecton modified the milestones: v1.0, v0.5.0 Mar 1, 2023
@ecton
Copy link
Member Author

ecton commented Mar 1, 2023

This also affects websocket connections. Given the idea of #275, I wonder if this request and it shouldn't be merged into one shared "system".

@ecton ecton modified the milestones: v0.5.0, v1.0 Mar 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request networking Issues relating to either the networked server or client
Projects
None yet
Development

No branches or pull requests

2 participants