New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
8256459: java/net/httpclient/ManyRequests.java and java/net/httpclient/LineBodyHandlerTest.java fail infrequently with java.net.ConnectException: Connection timed out: no further information #1716
Conversation
…t/LineBodyHandlerTest.java fail infrequently with java.net.ConnectException: Connection timed out: no further information Tests are updated to allow the test server to handle requests concurrently. PlainHttpConnection is updated to retry connection once if chan::finishConnect fails early with ConnectionException and the connection timeout has not expired.
|
Webrevs
|
@@ -187,7 +201,43 @@ public void abort(IOException ioe) { | |||
debug.log("Failed to close channel after unsuccessful connect"); | |||
} | |||
} | |||
return cf; | |||
return cf.handle((r,t) -> checkRetryConnect(r, t,exchange)) | |||
.thenCompose(Function.identity()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So this changes behaviour so that a single subsequent additional TCP connection may be tried for all cases where the connection fails AND there is remaining connection deadline. Ok.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes - I didn't want to depend on the exception message - and there's no way to distinguish with the exception type (it's raw ConnectionException
). It will however happen only when the ConnectionException
is thrown by SocketChannel::finishConnect
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ChrisHegarty I have modified the logic to not retry if jdk.httpclient.disableRetryConnect
was specified. By default that property is not specified and so we will retry once, but if an application has specifically opted for not retrying on ConnectException then we will not retry.
@dfuch This change now passes all automated pre-integration checks. After integration, the commit message for the final commit will be:
You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed. At the time when this comment was updated there had been 8 new commits pushed to the
Please see this link for an up-to-date comparison between the source branch of this pull request and the
|
…t/LineBodyHandlerTest.java fail infrequently with java.net.ConnectException: Connection timed out: no further information Connect will not be retried if retry connect is disabled JBS issue label noreg-self removed and bug id added to tests
/integrate |
@dfuch Since your change was applied there have been 22 commits pushed to the
Your commit was automatically rebased without conflicts. Pushed as commit 4a839e9. |
Hi,
Please find here a changeset that fixes the infrequent (but annoying) test failures
caused by unexpected ConnectionException "Connection timed out: no further information"
which have been observed to occur on some platforms.
Tests are updated to allow the test server to handle requests concurrently.
PlainHttpConnection is updated to retry connection once if chan::finishConnect fails
early with ConnectionException and the connection timeout has not expired.
Progress
Issue
Reviewers
Download
$ git fetch https://git.openjdk.java.net/jdk pull/1716/head:pull/1716
$ git checkout pull/1716