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

Client should have configurable timeout options #297

Closed
ecton opened this issue May 9, 2023 · 0 comments
Closed

Client should have configurable timeout options #297

ecton opened this issue May 9, 2023 · 0 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 May 9, 2023

Before BonsaiDb supported its blocking APIs, there was a way to cancel requests: use tokio::timeout. However, that isn't very ergonomic, and now that there is a blocking API, there is no way to ensure that an API request doesn't hang for a very long time.

The Client should have at least two timeout settings:

  • Connect Timeout
  • Request Timeout

Normally I would argue for an idle timeout, but #116 will address idle connections.

Because different code may want different timeout configurations, the request timeout setting should be modifiable on each Client instance. Because the connection task is shared between all Client instances, the connect timeout can only be set on the client builder.

One thing we should consider: if a request times out, should we try to send a request cancellation to the server? The server's current request processing may make it tricky to support cancellation of tasks, but it might be something to consider to help for a situation where the reason requests are timing out is because of a server being overloaded.

Thank you @phantie in #296 for reporting this deficiency!

@ecton ecton added enhancement New feature or request networking Issues relating to either the networked server or client labels May 9, 2023
@ecton ecton added this to the v1.0 milestone May 9, 2023
ecton added a commit that referenced this issue May 16, 2023
Refs #296, #297

This commit adds timeout settings for connections and requests. These
timeouts are separate, meaning that if the initial request takes X
seconds to connect, the request timeout will be timed in addition to
those X seconds.

This should be unit-tested, and the wasm client changes haven't been
tested at all beyond ensuring compilation.
@ecton ecton closed this as completed in bd862dd May 16, 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

1 participant