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

Document potential for timeouts on stream read/write #26

Closed
akshayjshah opened this issue Sep 2, 2021 · 1 comment
Closed

Document potential for timeouts on stream read/write #26

akshayjshah opened this issue Sep 2, 2021 · 1 comment

Comments

@akshayjshah
Copy link
Member

akshayjshah commented Sep 2, 2021

The net/http (and ultimately the net.Conn) APIs don't let us set the sort of timeouts we'd like for read and write operations. At the net.Conn level, we'd like something like an idle timeout: "if no data is sent or received for N ms, close the connection". Sadly, these APIs don't exist.

We're stuck with a timeout for the whole stream (set when constructing the http.Server). Any read or write can block, potentially until the whole stream times out. This opens streaming handlers up to abuse by clients who start a stream, but then don't send any data - until the whole stream times out, they'll tie up resources on the server. We could try to work around this by letting streams access their underlying net.Conn and setting a new deadline before every read or write, but that doubles the number of syscalls and is totally gross. (Not the most technical argument, but it's true.)

We should document this carefully, so that people using streaming RPCs know what they're signing up for. They may want to keep streaming handlers on a separate http.Server with longer timeouts, they may decide to avoid them completely in public APIs, or they may decide to wrap the default DialContext function on their server to set deadlines on each read/write.

FWIW, grpc-go and the standard library both have issues tracking this exact issue: grpc/grpc-go#1229 and golang/go#16100.

@akshayjshah akshayjshah mentioned this issue Jan 28, 2022
13 tasks
@akshayjshah akshayjshah added this to the Connect Launch milestone Mar 3, 2022
@akshayjshah
Copy link
Member Author

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant