Skip to content
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

Enable Http2_MultipleConnectionsEnabled_InfiniteRequestsCompletelyBlockOneConnection_RemaningRequestsAreHandledByNewConnection test #54683

Conversation

alnikola
Copy link
Contributor

@alnikola alnikola commented Jun 24, 2021

Fixes #45204

@ghost
Copy link

ghost commented Jun 24, 2021

Tagging subscribers to this area: @dotnet/ncl
See info in area-owners.md if you want to be subscribed.

Issue Details
Author: alnikola
Assignees: -
Labels:

area-System.Net.Http

Milestone: -

@alnikola alnikola added this to the 6.0.0 milestone Jun 24, 2021
@alnikola
Copy link
Contributor Author

Test Http2_MultipleConnectionsEnabled_ConnectionLimitNotReached_ConcurrentRequestsSuccessfullyHandled successfully completed 60000 iterations on Ubuntu 20.04 under WSL2 on a local machine.

@wfurt
Copy link
Member

wfurt commented Jun 24, 2021

did you run whole set or just the single test @alnikola ?

@alnikola
Copy link
Contributor Author

I ran only this test but through [MemberData].

@geoffkizer
Copy link
Contributor

These will likely be affected by my connection pooling changes.

If you want, I can just re-enable them in my PR (if they look clean on my machine).

@alnikola
Copy link
Contributor Author

@geoffkizer I'd enable them in a separate PR to make issues backtracking easier. But if you'd like it more, then go ahead and enable it in your PR.

@wfurt
Copy link
Member

wfurt commented Jun 24, 2021

anything flaky needs to be run as whole bundle @alnikola. Many tests would always pass when executed in isolation but may fail when completing for resources with other test.

@geoffkizer
Copy link
Contributor

@alnikola Makes sense, go ahead and enable them.

@alnikola
Copy link
Contributor Author

alnikola commented Jun 24, 2021

@wfurt Are talking about running only the test class owning this test (i.e. SocketsHttpHandlerTest_Http2) or all of tests in System.Net.Http.FunctionalTests project in a loop?

@geoffkizer
Copy link
Contributor

What about Http2_MultipleConnectionsEnabled_IdleConnectionTimeoutExpired_ConnectionRemovedAndNewCreated also?

@alnikola
Copy link
Contributor Author

Test Http2_MultipleConnectionsEnabled_InfiniteRequestsCompletelyBlockOneConnection_RemaningRequestsAreHandledByNewConnection also successfully completed 60000 iterations on Ubuntu 20.04 under WSL2 on a local machine.

@alnikola
Copy link
Contributor Author

What about Http2_MultipleConnectionsEnabled_IdleConnectionTimeoutExpired_ConnectionRemovedAndNewCreated also?

This test is failing locally. I'm investigating it.

@wfurt
Copy link
Member

wfurt commented Jun 24, 2021

I would run the whole System.Net.Http.FunctionalTests as CI would - just locally so it is more under controlled environment.

@alnikola
Copy link
Contributor Author

I would run the whole System.Net.Http.FunctionalTests as CI would - just locally so it is more under controlled environment.

OK, it makes sense. I will try it.

@alnikola
Copy link
Contributor Author

I would run the whole System.Net.Http.FunctionalTests as CI would - just locally so it is more under controlled environment.

I ran the whole System.Net.Http.FunctionalTests test set locally for ~30 mins and everything completed successfully.

@alnikola
Copy link
Contributor Author

/azp run runtime-libraries-coreclr

@azure-pipelines
Copy link

No pipelines are associated with this pull request.

@alnikola
Copy link
Contributor Author

/azp run runtime-libraries-coreclr innerloop

@azure-pipelines
Copy link

No pipelines are associated with this pull request.

@alnikola
Copy link
Contributor Author

Unfortunately, Http2_MultipleConnectionsEnabled_ConnectionLimitNotReached_ConcurrentRequestsSuccessfullyHandled now failed in CI on Windows, thus we cannot enable it right now.

System.Net.Http.Functional.Tests.SocketsHttpHandlerTest_Http2.Http2_MultipleConnectionsEnabled_ConnectionLimitNotReached_ConcurrentRequestsSuccessfullyHandled(a: 10) [FAIL]
      System.Threading.Tasks.TaskCanceledException : The request was canceled due to the configured HttpClient.Timeout of 100 seconds elapsing.
      ---- System.TimeoutException : The operation was canceled.
      -------- System.Threading.Tasks.TaskCanceledException : The operation was canceled.
      Stack Trace:
        /_/src/libraries/System.Net.Http/src/System/Net/Http/HttpClient.cs(618,0): at System.Net.Http.HttpClient.HandleFailure(Exception e, Boolean telemetryStarted, HttpResponseMessage response, CancellationTokenSource cts, CancellationToken cancellationToken, CancellationTokenSource pendingRequestsCts)
        /_/src/libraries/System.Net.Http/src/System/Net/Http/HttpClient.cs(541,0): at System.Net.Http.HttpClient.<SendAsync>g__Core|83_0(HttpRequestMessage request, HttpCompletionOption completionOption, CancellationTokenSource cts, Boolean disposeCts, CancellationTokenSource pendingRequestsCts, CancellationToken originalCancellationToken)
        /_/src/libraries/System.Net.Http/tests/FunctionalTests/SocketsHttpHandlerTest.cs(2287,0): at System.Net.Http.Functional.Tests.SocketsHttpHandlerTest_Http2.PrepareConnection(Http2LoopbackServer server, HttpClient client, UInt32 maxConcurrentStreams, Int32 readTimeout, Int32 expectedWarmUpTasks)
        /_/src/libraries/System.Net.Http/tests/FunctionalTests/SocketsHttpHandlerTest.cs(2070,0): at System.Net.Http.Functional.Tests.SocketsHttpHandlerTest_Http2.Http2_MultipleConnectionsEnabled_ConnectionLimitNotReached_ConcurrentRequestsSuccessfullyHandled(Int32 a)
        --- End of stack trace from previous location --

@alnikola
Copy link
Contributor Author

However, it seems that the test job itself timed out and was terminated, so it might be it wasn't an actual test failure.

@alnikola
Copy link
Contributor Author

I will enable only Http2_MultipleConnectionsEnabled_InfiniteRequestsCompletelyBlockOneConnection_RemaningRequestsAreHandledByNewConnection because others are failing.

@alnikola alnikola changed the title Trying to enable 2 disabled multiple HTTP/2 connection tests Enable Http2_MultipleConnectionsEnabled_InfiniteRequestsCompletelyBlockOneConnection_RemaningRequestsAreHandledByNewConnection test Jun 28, 2021
@alnikola alnikola requested a review from a team June 28, 2021 16:33
@alnikola alnikola marked this pull request as ready for review June 28, 2021 16:33
@alnikola alnikola merged commit 72047c5 into dotnet:main Jun 29, 2021
@alnikola alnikola deleted the alnikola/41078-45204-enable-2-multi-http2-conn-tests branch June 29, 2021 11:51
@ghost ghost locked as resolved and limited conversation to collaborators Jul 29, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
4 participants