azure-sdk-for-cpp icon indicating copy to clipboard operation
azure-sdk-for-cpp copied to clipboard

Can we increase receive timeout for WinHTTP transport?

Open Jinming-Hu opened this issue 2 years ago • 8 comments

The default receive timeout for WinHTTP connections is 30 seconds. Storage service usually takes longer than that to respond to Query requests. As a result, client usually needs to retry 3 or 4 times to get a response within 30 seconds. This wastes quite some time and bandwidth.

So we'd like to see if we can increase the timeout to a larger value, or expose this as an option so that client can customize as needed.

As a reference, libcurl doesn't timeout once the connection is established.

REF: https://docs.microsoft.com/en-us/windows/win32/api/winhttp/nf-winhttp-winhttpsettimeouts https://curl.se/libcurl/c/CURLOPT_TIMEOUT.html

Jinming-Hu avatar May 11 '22 13:05 Jinming-Hu

Retried 3 times to get a HTTP response that returned within 30 seconds. image

request timeline:

Request Count:   1
Bytes Sent:      1,076		(headers:561; body:515)
Bytes Received:  33,915,479		(headers:646; body:33,914,833)

ACTUAL PERFORMANCE
--------------
ClientConnected:	20:30:37.112
ClientBeginRequest:	20:30:38.258
GotRequestHeaders:	20:30:38.258
ClientDoneRequest:	20:30:38.259
Determine Gateway:	0ms
DNS Lookup: 		0ms
TCP/IP Connect:	0ms
HTTPS Handshake:	0ms
ServerConnected:	20:30:37.119
FiddlerBeginRequest:	20:30:38.259
ServerGotRequest:	20:30:38.259
ServerBeginResponse:	20:31:18.115             <- 40 seconds after getting request
GotResponseHeaders:	20:31:18.115
ServerDoneResponse:	20:31:27.660
ClientBeginResponse:	20:31:27.669
ClientDoneResponse:	20:31:27.669

	Overall Elapsed:	0:00:49.411

RESPONSE BYTES (by Content-Type)
--------------
avro/binary: 33,914,833
  ~headers~: 646

Jinming-Hu avatar May 11 '22 13:05 Jinming-Hu

@Jinming-Hu, what would you see the timeout option too, specifically for query APIs, given 30 seconds is too short? Does the service has a guaranteed upper bound for how long it will take to respond?

ahsonkhan avatar May 12 '22 16:05 ahsonkhan

There's also a difference in the default connection timeouts between the curl transport adapter (we set to 5 min) and winhttp (set to 1 min).

ahsonkhan avatar May 12 '22 16:05 ahsonkhan

@ahsonkhan I didn't find SLA about response time. I think 3 minutes is a reasonable time. It's not too long, and it's long enough for Query.

Connection timeout doesn't break storage at the moment, so I'm fine to leave it as is.

Jinming-Hu avatar May 12 '22 23:05 Jinming-Hu

I found this link. We don't have SLA for Query API yet.

Jinming-Hu avatar May 13 '22 10:05 Jinming-Hu

Bump up this ask from Azure Storage service team. We are exploring to consume Azure Storage CPP SDK inside backend service. However, there is a requirement to fine grained control HTTP timeouts settings. For example, 4 kinds of timeout settings to be consumed by upper layer.

   WinHttpSetTimeouts(
        requestHandle,
        timeoutInMs /* ResolveTimeout */,
        timeoutInMs /* ConnectTimeout */,
        timeoutInMs /* SendTimeout */,
        timeoutInMs /* ReceiveTimeout */);

XiaoningLiu avatar Aug 19 '22 03:08 XiaoningLiu

@RickWinter @ahsonkhan Can you prioritize this feature request? We have different cases that require customizable timeouts for transport layers.

Jinming-Hu avatar Aug 19 '22 03:08 Jinming-Hu

It's possible that this can increase the receive response timeout.

this can be used to increase the connect timeout. this can be used to increase the DNS resolution timeout. and finally this can be used to set the send timeout.

We can expose all 4 of these timeouts in seconds in the WinHTTP client options which storage can then use if it needs to modify the WinHTTP options.

Exposing WInHttpSetTimeouts seems like a poor choice since it forces all the values to be set explicitly.

LarryOsterman avatar Mar 12 '24 06:03 LarryOsterman

Hi @Jinming-Hu, we deeply appreciate your input into this project. Regrettably, this issue has remained unresolved for over 2 years and inactive for 30 days, leading us to the decision to close it. We've implemented this policy to maintain the relevance of our issue queue and facilitate easier navigation for new contributors. If you still believe this topic requires attention, please feel free to create a new issue, referencing this one. Thank you for your understanding and ongoing support.

github-actions[bot] avatar May 10 '24 18:05 github-actions[bot]