Issue 751 #859
Closed
Issue 751 #859
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
The TEST_END message is racing with the server_timer_proc timer. When the RTT is higher than a second, the timer wins the race and closes the control socket before the results are exchanged. This results in the client reporting: "error - control socket has closed unexpectedly" as reported in GH issue 751. This change doesn't prevent the race, but significantly increases the grace period based on a maximum RTT of 4 seconds and accounts for the ten transitions in the iperf3 state machine.
When the server receives IPERF_DONE, clean up and close the control socket, instead of waiting for the test timer to expire.
|
Thanks for the PR! I hope to get around to studying the code change sometime soon...your description of the problem and solution make sense given the symptoms that have been reported. |
This was referenced Jun 12, 2019
bmah888
reviewed
Jun 12, 2019
There was a problem hiding this comment.
This looks useful, thanks. I might have made this something that was settable with a command-line parameter on the server (with a long default), and not bothering to derive the value from the rtt and number of state transitions. I'm not saying you need to change it for this pull request, I'm mostly just thinking "out loud".
bmah888
requested changes
Jun 12, 2019
bmah888
pushed a commit
that referenced
this pull request
Jun 14, 2019
The TEST_END message is racing with the server_timer_proc timer. When the RTT is higher than a second, the timer wins the race and closes the control socket before the results are exchanged. This results in the client reporting: "error - control socket has closed unexpectedly" as reported in GH issue 751. This change doesn't prevent the race, but significantly increases the grace period based on a maximum RTT of 4 seconds and accounts for the ten transitions in the iperf3 state machine. (cherry picked from commit 34bdddb) (originally submitted by @acooks in #859)
|
Because I'm trying to finish off some things, I've just gone ahead and cherry-picked the first of the two commits from this pull-request to the iperf3 master branch. Thanks for the pull-request! |
|
Ok. I believe the second commit is essential, but haven't had time to trace
the path that demonstrates that point again. It should be somewhat obvious
that the server takes longer to close the connection and cleanup between
multiple runs in a simple manual test.
…On Sat., 15 Jun. 2019, 04:42 Bruce A. Mah, ***@***.***> wrote:
Because I'm trying to finish off some things, I've just gone ahead and
cherry-picked the first of the two commits from this pull-request to the
iperf3 master branch. Thanks for the pull-request!
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#859?email_source=notifications&email_token=AADQJN3ABRUXBNEJVMZ44RLP2PRB7A5CNFSM4HHDD36KYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODXXUQVA#issuecomment-502220884>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AADQJN37J2H4FAQKG7IAOVTP2PRB7ANCNFSM4HHDD36A>
.
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in 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.
Fixes issue v3.5 error - control socket has closed unexpectedly #751
Reduce likelihood of triggering server cleanup timeout before results can be exchanged, which happens when the round trip time is hundreds to thousands of milliseconds.
This change is