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

embed: wait up to request-timeout for pending RPCs when closing #8267

Merged
merged 2 commits into from
Jul 15, 2017

Conversation

gyuho
Copy link
Contributor

@gyuho gyuho commented Jul 14, 2017

Both grpc.Server.Stop and grpc.Server.GracefulStop close the listeners
first, to stop accepting the new connections. GracefulStop blocks until
all clients close their open transports(connections). Unary RPCs
only take a few seconds to finish. Stream RPCs, like watch, might never
close the connections from client side, thus making gRPC server wait
forever.

This patch still calls GracefulStop, but waits up to 10s before manually
closing the open transports.

Address #8224.

@heyitsanthony #8224 (comment)

Is there some way the server can stop the client listener and cut the connections instead of waiting for all clients to disconnect on their own?

This stops all client listeners first (no more new RPCs), and wait up to request timeout before cutting existing transports, to give enough time for unary RPCs to be finished.

@heyitsanthony
Copy link
Contributor

test case?

embed/etcd.go Outdated
// wait until all pending RPCs are finished
select {
case <-ch:
case <-time.After(10 * time.Second):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe use the election timeout? 10s seems too generous

@gyuho gyuho added the WIP label Jul 14, 2017
@gyuho gyuho changed the title embed: wait up to 10s for pending RPCs when closing embed: wait up to election-timeout for pending RPCs when closing Jul 14, 2017
@gyuho gyuho force-pushed the close-server branch 2 times, most recently from 29355cd to 6bd7522 Compare July 14, 2017 21:53
@gyuho gyuho removed the WIP label Jul 14, 2017
Both grpc.Server.Stop and grpc.Server.GracefulStop close the listeners
first, to stop accepting the new connections. GracefulStop blocks until
all clients close their open transports(connections). Unary RPCs
only take a few seconds to finish. Stream RPCs, like watch, might never
close the connections from client side, thus making gRPC server wait
forever.

This patch still calls GracefulStop, but waits up to 10s before manually
closing the open transports.

Address etcd-io#8224.

Signed-off-by: Gyu-Ho Lee <gyuhox@gmail.com>
Ensure 'Close' returns in time when there are open
connections (watch streams).

Signed-off-by: Gyu-Ho Lee <gyuhox@gmail.com>
@gyuho gyuho changed the title embed: wait up to election-timeout for pending RPCs when closing embed: wait up to request-timeout for pending RPCs when closing Jul 14, 2017
@heyitsanthony
Copy link
Contributor

lgtm once tests pass. Should this be backported?

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

Successfully merging this pull request may close these issues.

2 participants