rpc/grpc: Perform read-write message operations with timeout #366
Conversation
Codecov Report
@@ Coverage Diff @@
## master #366 +/- ##
==========================================
- Coverage 61.74% 61.69% -0.05%
==========================================
Files 66 66
Lines 10043 10051 +8
==========================================
Hits 6201 6201
- Misses 2900 2908 +8
Partials 942 942
Continue to review full report at Codecov.
|
| func (w *streamWrapper) withTimeout(closure func() error) error { | ||
| ch := make(chan error, 1) | ||
| go func() { | ||
| ch <- closure() |
There was a problem hiding this comment.
We should close(ch) after this.
| return w.withTimeout(w.ClientStream.CloseSend) | ||
| } | ||
|
|
||
| func (w *streamWrapper) withTimeout(closure func() error) error { |
There was a problem hiding this comment.
Have you checked the performance impact of this for small messages? E.g. object.Put uses stream.
There was a problem hiding this comment.
Tried it on unary object.Head. Without business logic (no signature checks and metabase access, pure gRPC communication over localhost), there is a 13-15% overhead. It is worst case scenario, so I expect it will be much less noticeable in deploy environment.
goos: linux
goarch: amd64
pkg: github.com/nspcc-dev/neofs-node/cmd/example/client
cpu: Intel(R) Core(TM) i7-8550U CPU @ 1.80GHz
BenchmarkHeadRequests
BenchmarkHeadRequests-8 9391 112575 ns/op
BenchmarkHeadRequestsWithDeadline
BenchmarkHeadRequestsWithDeadline-8 8246 127810 ns/op
…timeout Remote gRPC server may not return or accept data for a while. gRPC solves this issue with timeout in context. However, the context is used for entire gRPC method invocation. Unfortunately the duration of requests with streams can't be estimated easily. To solve this issue we can specify timeouts for every message read and write. Single message has size limit so timeout can be related to that. Signed-off-by: Alex Vanin <[email protected]>
Signed-off-by: Alex Vanin <[email protected]>
Remote gRPC server may not return or accept data for a while. gRPC solves this issue with timeout in context. However, the context is used for entire gRPC method invocation. Unfortunately the duration of requests with streams can't be estimated easily.
To solve this issue we can specify timeouts for every message read and write. Single message has size limit so timeout can be related to that.