fix: rework the context object in gRPC calls

[aleksander-vedvik]

There is a workaround for the context object due to how a stream is used as the underlying communication. This workaround makes the stream context coupled with individual gRPC contexts.

To make it easier to follow, we will make a distinction between the two contexts:

• streamCtx: The context created when the client connects to the server, i.e. at the initialization of NodeStream.
• rpcCtx: The context created by the client and provided to a gRPC invocation.

The current implementation is as follows:

• The streamCtx is created at the initialization of the NodeStream.
• An rpcCtx is created when the client wants to invoke a gRPC method on the server.
• If the client cancels the rpcCtx then Gorums will cancel the streamCtx causing the stream to be closed. This is done in channel.go:

  go func() {
  select {
  case <-done:
    // all is good
  case <-req.ctx.Done():
    // Both channels could be ready at the same time, so we should check 'done' again.
    select {
    case <-done:
        // false alarm
    default:
        // cause reconnect
        c.cancelStream()
    }
  }
  }()

• Gorums will then return the error and the stream will be down. This is done in channel.go:

  // else try to send message
  err := c.sendMsg(req)
  if err != nil {
  // return the error
  c.routeResponse(req.msg.Metadata.MessageID, response{nid: c.node.ID(), msg: nil, err: err})
  }

To be discussed:

• Cancelling the rpcCtx will cause the streamCtx to be cancelled, thus terminating the stream. There are no mechanisms in place to reconnect when this happens. Meaning a new configuration has to be created.
• Rework the rpcCtx to be independent of the streamCtx.
  • Maybe it would be necessary to create a custom context and send it with each message?
  • A potential benefit would be context propagation.
• Create a benchmark to identify the overhead of the current solution.

[meling]

I agree that the rpcCtx should be independent of the streamCtx. The rpcCtx should be passed along with each message over the NodeStream. The streamCtx should not be exposed to the client developer at all. That said, if there is a benefit of using the streamCtx when closing a connection (e.g., for clean shutdown), that should be fine. But the client should not have the option to cancel or set a timeout on the NodeStream's streamCtx via an RPC/Quorum call.

@johningve I don't recall if we discussed the above previously, but please let us know if you have some additional context related to this that could be useful for us to know about. Thanks!