Timeouts for CS, SS, DS should mean the time to the initial message response from the server side

[amydevs]

Specification

Timeouts for Streaming Calls should only be until the initial message. Once the timeout has been fulfilled, there should be no timeouts that affect the call after that point.

Client

For the client, the timeout is the time that it will wait until getting the first message from the peer.

Server

The timeout will stay advisory for servers, so the server can throw optionally as they need. Timeouts are now understood as the time the client will wait for until the first message.

Additional context

Tasks

1. [x] Apply timeout changes to RPCClient
2. [x] Apply timeout changes to RPCServer
3. [x] Rewrite tests for new timeouts

[CMCDragonkai]

So the README.md was updated, but actually implementation wasn't done.

The point is that timeouts for RPC when it comes to CS, SS, DS shouldn't be per-message timeouts. And they shouldn't be per-call timeouts.

The only timeout that makes sense here, is timeout to FIRST response.

THUS:

1. RPCClient timeout means that for unary, CS, SS, DS, means it is the time I will wait for until I get the first message from the other side whatever that is. For unary this behaviour is the same as is, for the others it allows long-running RPC streams.
2. RPCServer timeout is advisory, thus handlers can decide what to do here. But generally speaking it should be understood as the time the client will wait for until the first message. For unary this is the same meaning. For the others, if the timeout is exhausted, and I haven't sent the initial message response, then it should throw. But if it is the case I have already responded, then I don't need to worry about the timer anymore. This has implications on the implementation of the SS, CS, DS handlers, since they shouldn't be passing the ctx down to any calls UNLESS it's part of the process before the initial response message. Otherwise, handlers may still maintain its own timers when executing things it expects to take some time to yield a result. YMMV.

@amydevs I need the tasks fleshed out here.

Will this impact the #27?

[CMCDragonkai]

In the future @amydevs do not write generic issue titles. Issues must be specific!

[CMCDragonkai]

In relation to Node Connections, we would say that as long as the RPC stream is live, then NCs should not expire. Because it could mean there's a long running RPC call happening, and this is true even if there is no RPC activity (no messages). At the bottom in the transport layer, there can still be keep alive packets, these are not the same thing.

@tegefaulkes can you confirm this?

[tegefaulkes]

The TTL timer for the NodeConnections in the connection map is disable when the connection is being used to make a call. That's if there is a withConnF being used.

I also added this logic to the reverse streams in #609. So yeah, if the connection is actively being used then it will not time out from the TTL.

[amydevs]

So the README.md was updated, but actually implementation wasn't done.

The point is that timeouts for RPC when it comes to CS, SS, DS shouldn't be per-message timeouts. And they shouldn't be per-call timeouts.

The only timeout that makes sense here, is timeout to FIRST response.

THUS:

1. RPCClient timeout means that for unary, CS, SS, DS, means it is the time I will wait for until I get the first message from the other side whatever that is. For unary this behaviour is the same as is, for the others it allows long-running RPC streams.

2. RPCServer timeout is advisory, thus handlers can decide what to do here. But generally speaking it should be understood as the time the client will wait for until the first message. For unary this is the same meaning. For the others, if the timeout is exhausted, and I haven't sent the initial message response, then it should throw. But if it is the case I have already responded, then I don't need to worry about the timer anymore. This has implications on the implementation of the SS, CS, DS handlers, since they shouldn't be passing the ctx down to any calls UNLESS it's part of the process before the initial response message. Otherwise, handlers may still maintain its own timers when executing things it expects to take some time to yield a result. YMMV.

@amydevs I need the tasks fleshed out here.

Will this impact the #27?

I don't think should impact #27

[CMCDragonkai]

The TTL timer for the NodeConnections in the connection map is disable when the connection is being used to make a call. That's if there is a withConnF being used.

I also added this logic to the reverse streams in #609. So yeah, if the connection is actively being used then it will not time out from the TTL.

And by actively used you mean it will support RPC streams that have no activity yet but the stream is still alive.

[tegefaulkes]

Yes.