http: Revert the default timeout to 60 seconds #74
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
bennyz
previously approved these changes
May 24, 2022
aesteve-rh
reviewed
May 24, 2022
vjuranek
previously approved these changes
May 25, 2022
In commit d5e9c75 (http: Configurable inactivity timeout) we changed the default timeout from 60 seconds to 15. The reasoning was that clients have no reason to connect and keep the connection idle for long time. Once a client connects, it is expected to start sending requests. On the first request, the socket timeout is replaced by the ticket inactivity timeout, set by the user creating the transfer. Turns out that there is a valid use case for idle clients, and the shorter timeout breaks downloads of big images (reproduced with 8 TiB image. The failure flow is: 1. Client connects and send an EXTENTS request. 2. While EXTENTS request is collecting data, client connects multiple downloads connections. 3. The connected download threads wait on a queue for work, but since EXTENTS request did not finish, the connections are idle. 4. After 15 seconds the server close the idle connections. 5. When the EXTENTS request finish, the client fails to send request to the server. Here is example failure on the client side from ovirt-stress backup run: ... File "/usr/lib64/python3.6/site-packages/ovirt_imageio/_internal/io.py", line 288, in copy self._src.write_to(self._dst, req.length, self._buf) File "/usr/lib64/python3.6/site-packages/ovirt_imageio/_internal/backends/http.py", line 207, in write_to res = self._get(length) File "/usr/lib64/python3.6/site-packages/ovirt_imageio/_internal/backends/http.py", line 432, in _get self._con.request("GET", self.url.path, headers=headers) File "/usr/lib64/python3.6/http/client.py", line 1273, in request self._send_request(method, url, body, headers, encode_chunked) File "/usr/lib64/python3.6/http/client.py", line 1319, in _send_request self.endheaders(body, encode_chunked=encode_chunked) File "/usr/lib64/python3.6/http/client.py", line 1268, in endheaders self._send_output(message_body, encode_chunked=encode_chunked) File "/usr/lib64/python3.6/http/client.py", line 1044, in _send_output self.send(msg) File "/usr/lib64/python3.6/http/client.py", line 1004, in send self.sock.sendall(data) BrokenPipeError: [Errno 32] Broken pipe Looking in the server logs, we can see that EXTENTS request took about 24 seconds: 2022-05-24 16:22:57,548 INFO (Thread-75) [extents] [local] EXTENTS transfer=39f14719-2533-45d5-8315-9d0b577d5732 context=zero ... 2022-05-24 16:23:44,907 INFO (Thread-75) [http] CLOSE connection=75 client=local [connection 1 ops, 47.359750 s] [dispatch 2 ops, 47.297816 s] [extents 2 ops, 47.296568 s] Downloading 8 TiB disk is an edge case, but this can happen with smaller images on very fragmented file system, or if there is another reason that cause EXTENTS request to be slow. Revert the timeout back to the previous value used in ovirt 4.4. We may shorten the timeout once we support partial extents: https://bugzilla.redhat.com/1924940 Fixes: oVirt#71 Signed-off-by: Nir Soffer <nsoffer@redhat.com>
aesteve-rh
approved these changes
May 25, 2022
# for free
to join this conversation on GitHub.
Already have an account?
# to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
In commit d5e9c75 (http: Configurable inactivity timeout) we
changed the default timeout from 60 seconds to 15. The reasoning was
that clients have no reason to connect and keep the connection idle for
long time. Once a client connects, it is expected to start sending
requests. On the first request, the socket timeout is replaced by the
ticket inactivity timeout, set by the user creating the transfer.
Turns out that there is a valid use case for idle clients, and the
shorter timeout breaks downloads of big images (reproduced with 8 TiB
image. The failure flow is:
downloads connections.
EXTENTS request did not finish, the connections are idle.
the server.
Here is example failure on the client side from ovirt-stress backup run:
Looking in the server logs, we can see that EXTENTS request took about
24 seconds:
Downloading 8 TiB disk is an edge case, but this can happen with smaller
images on very fragmented file system, or if there is another reason
that cause EXTENTS request to be slow.
Revert the timeout back to the previous value used in ovirt 4.4.
We may shorten the timeout once we support partial extents:
https://bugzilla.redhat.com/1924940
Fixes #71
Signed-off-by: Nir Soffer nsoffer@redhat.com