OctopusDeploy / Halibut

| Public | A secure communication stack for .NET using JSON-RPC over SSL.
Other
12 stars 44 forks source link

Timing out faster after initial request data has been received. #546

Closed sburmanoctopus closed 12 months ago

sburmanoctopus commented 12 months ago

[sc-42848]

Background

Currently, we have a large timeout of 10 minutes for all TCP responses, because we need enough time for the Tentacle to perform the RPC itself.

But once the RPC has finished, and we start receiving the data, we should be able to drastically reduce the read timeout.

Results

Before

Once the response data starts streaming back, we would continue using the large timeout value (which factors in the time taken to call the RPC).

After

We have now made 2 new timeout values:

These are both specific to waiting for a response. The first is used when waiting for the initial data to be returned (and includes the time to execute the RPC). The second is the timeout used when data has started returning.

Default Values

The two new timeouts have both been set to 10 minutes each, as that is the current timeout that would be used.

We will make a separate PR for Octopus Server using the existing feature toggle to trial the new values we wish to use. When these work successfully, we will apply them to HalibutTimeoutsAndLimits permanently.

RewindableBufferStream

A bug was found with RewindableBufferStream, where it would discard any data in its buffer when StartBuffer was called again. This is not an issue with the existing code-base, as we never called StartBuffer a second time. But now that we do, we had to this in this PR.

WithTimeout

The initial version used the existing WithTimeout method using the SendReceiveTimeout. But since this change only affected the "read" timeout, it meant creating a value for "send" timeout that wasn't used. So we made WithReadTimeout

How to review this PR

Quality :heavy_check_mark:

Pre-requisites

shortcut-integration[bot] commented 12 months ago

This pull request has been linked to Shortcut Story #42848: Significantly reduce the current TCP Timeout of 10 minutes (not control messages).