Server side timeouts with goland

[stu...@stuartbishop.net]

Hi.

Is there a preferred way of implementing server side timeouts with the generated Go gRPC code? I'm using bidirectional streaming, with the extended request tied to a database transaction. I need to ensure database transactions are not held open more than a few seconds.

Doing this in Go, my best option seems to be to create a new context using context.WithTimeout(stream.Context(), dur) and using that. If I use this context when beginning the transaction, it will cause the transaction to rollback on the timeout. This does mean that the gRPC handler may keep running for some time, blocked on a stream.Recv or similar. So I can also use a goroutine for the main body of my handler, and wait of either the goroutine to complete or my context to be cancelled. Technically this would leak the goroutine I think (if blocked on stream.Recv or similar), although I think all that should clean up when the stream.Context() gets cancelled.

An alternative might be to wrap the stream, using channels for sending and receiving messages rather than blocking calls, and allowing me to block until either a message is sent/received or the context cancelled. But I would either need to wrap every generated ServerStream or lose compile time type checking. I still get (I think benign) leaked goroutines as above.

Or maybe there is something built in to the gRPC library that I missed?

[stu...@stuartbishop.net]

On Friday, 7 July 2017 17:11:01 UTC+7, stu...@stuartbishop.net wrote:

Hi.

Is there a preferred way of implementing server side timeouts with the generated Go gRPC code? I'm using bidirectional streaming, with the extended request tied to a database transaction. I need to ensure database transactions are not held open more than a few seconds.

Doing this in Go, my best option seems to be to create a new context using context.WithTimeout(stream.Context(), dur) and using that. If I use this context when beginning the transaction, it will cause the transaction to rollback on the timeout. This does mean that the gRPC handler may keep running for some time, blocked on a stream.Recv or similar. So I can also use a goroutine for the main body of my handler, and wait of either the goroutine to complete or my context to be cancelled. Technically this would leak the goroutine I think (if blocked on stream.Recv or similar), although I think all that should clean up when the stream.Context() gets cancelled.

Or maybe there is something built in to the gRPC library that I missed?

I've also tried installing an inTapHandle, which adds a timeout to the context. Unfortunately with this approach, it seems that this shuts down communication too early and no error returned to the client, which can leave a client hanging indefinitely trying to receive a message.

[Doug Fawley]

On Friday, July 7, 2017 at 3:11:01 AM UTC-7, stu...@stuartbishop.net wrote:

Doing this in Go, my best option seems to be to create a new context using context.WithTimeout(stream.Context(), dur) and using that. If I use this context when beginning the transaction, it will cause the transaction to rollback on the timeout.

I would recommend this.  I'm having a bit of trouble understanding where the problem comes in.  Are you performing these transactions in a goroutine, with the main service handler continually receiving requests and forking goroutines to handle them?

Technically this would leak the goroutine I think (if blocked on stream.Recv or similar), although I think all that should clean up when the stream.Context() gets cancelled.

Yes, stream.Recv() will unblock when the stream is terminated, so I wouldn't worry about this.

An alternative might be to wrap the stream, using channels for sending and receiving messages rather than blocking calls, and allowing me to block until either a message is sent/received or the context cancelled. But I would either need to wrap every generated ServerStream or lose compile time type checking. I still get (I think benign) leaked goroutines as above.

That seems reasonable, too.  Note that stream.Send and Recv aren't thread-safe, so you may only have one call of Send or Recv (each) outstanding at any time.  If you are performing multiple operations concurrently, you'll need to synchronize the sending of the results somehow already.

On Tuesday, July 11, 2017 at 2:26:21 AM UTC-7, stu...@stuartbishop.net wrote:

I've also tried installing an inTapHandle, which adds a timeout to the context. Unfortunately with this approach, it seems that this shuts down communication too early and no error returned to the client, which can leave a client hanging indefinitely trying to receive a message.

That behavior does not sound intentional.  If you have a reproducible case that demonstrates this, please file an issue on github.  The client should see the deadline exceeded error (but possibly with code=unknown).  Does this approach help with your problem, though?  Are you only performing one operation per RPC?  If you are trying to maintain a long-running stream and apply a shorter deadline to individual operations, then I don't think this would be appropriate, as the context returned applies to the entire stream, not each message.

Thanks,

Doug

[stu...@stuartbishop.net]

On Wednesday, 19 July 2017 23:41:56 UTC+7, Doug Fawley wrote:

On Friday, July 7, 2017 at 3:11:01 AM UTC-7, stu...@stuartbishop.net wrote:

Doing this in Go, my best option seems to be to create a new context using context.WithTimeout(stream.Context(), dur) and using that. If I use this context when beginning the transaction, it will cause the transaction to rollback on the timeout.

I would recommend this.  I'm having a bit of trouble understanding where the problem comes in.  Are you performing these transactions in a goroutine, with the main service handler continually receiving requests and forking goroutines to handle them?

No, it is just ping pong. The handler starts a transaction, then processes incomming messages one at a time, returning a result each time. With my own context, I do need to work around the blocking Send & Recv calls and run the main body of the handler in a separate goroutine so I can catch the timeout and fail.

Managing my own context this way works. It is annoying that I have to include this boiler plate in every handler, so it would be better if I could set the timeout on the context using the tap handler. Or perhaps the unary and stream intercepts is an option.

Technically this would leak the goroutine I think (if blocked on stream.Recv or similar), although I think all that should clean up when the stream.Context() gets cancelled.

Yes, stream.Recv() will unblock when the stream is terminated, so I wouldn't worry about this.

An alternative might be to wrap the stream, using channels for sending and receiving messages rather than blocking calls, and allowing me to block until either a message is sent/received or the context cancelled. But I would either need to wrap every generated ServerStream or lose compile time type checking. I still get (I think benign) leaked goroutines as above.

That seems reasonable, too.  Note that stream.Send and Recv aren't thread-safe, so you may only have one call of Send or Recv (each) outstanding at any time.  If you are performing multiple operations concurrently, you'll need to synchronize the sending of the results somehow already.

On Tuesday, July 11, 2017 at 2:26:21 AM UTC-7, stu...@stuartbishop.net wrote:

I've also tried installing an inTapHandle, which adds a timeout to the context. Unfortunately with this approach, it seems that this shuts down communication too early and no error returned to the client, which can leave a client hanging indefinitely trying to receive a message.

That behavior does not sound intentional.  If you have a reproducible case that demonstrates this, please file an issue on github.  The client should see the deadline exceeded error (but possibly with code=unknown).  Does this approach help with your problem, though?  Are you only performing one operation per RPC?  If you are trying to maintain a long-running stream and apply a shorter deadline to individual operations, then I don't think this would be appropriate, as the context returned applies to the entire stream, not each message.

I have filed https://github.com/grpc/grpc-go/issues/1386

I want the timeout applied to the stream. The transaction is opened at the start of the stream and used for each message being processed. A failure causes the whole transaction to rollback. If it successfully reaches EOF, the transaction is committed. The handler returns nil for the error, or an error if the commit failed due to deferred consistency checks or similar.