Reset the inactivity timeout when reusing connections #4164
Conversation
This file contains hidden or 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
bryancall
changed the title
bryancall
reviewed
Aug 27, 2018
proxy/http/HttpSM.cc
Outdated
| server_entry->write_vio = server_entry->vc->do_io_write(this, hdr_length, buf_start); | ||
|
|
||
| // Make sure the VCs is on the correct timeouts | ||
| server_session->get_netvc()->set_inactivity_timeout(HRTIME_SECONDS(t_state.txn_conf->transaction_no_activity_timeout_out)); |
Contributor
There was a problem hiding this comment.
Are we now setting the inactivity timeout multiple times for POST and PUSH?
Member
Author
There was a problem hiding this comment.
We could move the inactivity_timeout to setup_server_request_api. I think that would avoid the POST/PUSH case. Not sure if that would still hit the transform cases.
…when reusing connections.
shinrich
force-pushed
the
another-connect-timeout-fix
branch
from
a859e3b to
3ee2728
Compare
Member
Author
|
Updated commit to move the timeout reset to a path that should not affect POST/PUSH |
bryancall
approved these changes
Sep 19, 2018
Contributor
|
Cherry picked to 8.1.0 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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 9b47aa6 we fixed the logic to move from connect timeout to transaction no activity timeout after the new connection had completed. However, there was still a problem with sitting on the connection timeout until the first byte is received from the origin in the case of server connection reuse for GETs (not POST or PUSH). The setup for post requests were setting the inactivity timeout correctly but the set up for gets was not.
This has been running in production and verified.