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.

Sign up for GitHub

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

Jump to bottom

Enable Http2_MultipleConnectionsEnabled_ConnectionLimitNotReached_ConcurrentRequestsSuccessfullyHandled #55572

Merged
merged 2 commits into from
Jul 14, 2021
Merged

Enable Http2_MultipleConnectionsEnabled_ConnectionLimitNotReached_ConcurrentRequestsSuccessfullyHandled #55572

merged 2 commits into from
Jul 14, 2021

Conversation

alnikola
Copy link
Contributor

Fixes #41078

@ghost
Copy link

ghost commented Jul 13, 2021

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

Issue Details

Fixes #41078

Author: alnikola
Assignees: -
Labels:

area-System.Net.Http
Milestone: -

stephentoub
stephentoub reviewed Jul 13, 2021
View reviewed changes
src/libraries/System.Net.Http/tests/FunctionalTests/SocketsHttpHandlerTest.cs Outdated
{
yield return new object[] {i};
}
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is just for testing CI, right? You'll remove this and put it back to being a fact before merging?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, that's correct.

stephentoub
stephentoub approved these changes Jul 13, 2021
View reviewed changes
Copy link
Member

@stephentoub stephentoub left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the end change ends up just being removing the ActiveIssue and it passes CI, LGTM.

@alnikola alnikola marked this pull request as ready for review July 13, 2021 17:46
@stephentoub stephentoub merged commit 3e8ef85 into dotnet:main Jul 14, 2021
@jeffhandley
Copy link
Member

I just got a failure from this test in #46417. Log

    System.Net.Http.Functional.Tests.SocketsHttpHandlerTest_Http2.Http2_MultipleConnectionsEnabled_ConnectionLimitNotReached_ConcurrentRequestsSuccessfullyHandled [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(623,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(546,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(2303,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(2094,0): at System.Net.Http.Functional.Tests.SocketsHttpHandlerTest_Http2.Http2_MultipleConnectionsEnabled_ConnectionLimitNotReached_ConcurrentRequestsSuccessfullyHandled()
        --- End of stack trace from previous location ---

@jeffhandley jeffhandley mentioned this pull request Jul 14, 2021
Merged
@alnikola
Copy link
Contributor Author

I just got a failure from this test in #46417. Log

Let's revert it then.

@alnikola alnikola deleted the alnikola/41078-enable-conn-limit-not-reached branch July 14, 2021 11:51
alnikola added a commit that referenced this pull request Jul 14, 2021
@alnikola
Revert "Enable Http2_MultipleConnectionsEnabled_ConnectionLimitNotRea…
8a6fa30 
…ched_ConcurrentRequestsSuccessfullyHandled (#55572)"

This reverts commit 3e8ef85.
This was referenced Jul 14, 2021
Merged
Closed
thaystg added a commit to thaystg/runtime that referenced this pull request Jul 14, 2021
@thaystg
Merge branch 'main' of github.com:thaystg/runtime into thays_support_…
a8fb8cf 
…debugger_custom_views

* 'main' of github.com:thaystg/runtime: (125 commits)
  [wasm] [debugger] Support method calls  (dotnet#55458)
  [debugger] Fix debugging after hot reloading (dotnet#55599)
  Inliner: Extend IL limit for profiled call-sites, allow inlining for switches. (dotnet#55478)
  DiagnosticSourceEventSource supports base class properties (dotnet#55613)
  [mono] Fix race during mono_image_storage_open (dotnet#55201)
  [mono] Add wrapper info for native func wrappers. (dotnet#55602)
  H/3 and Quic AppContext switch (dotnet#55332)
  Compression.ZipFile support for Unix Permissions (dotnet#55531)
  [mono] Fix skipping of static methods during IMT table construction. (dotnet#55610)
  Combine System.Private.Xml TrimmingTests projects (dotnet#55606)
  fix name conflict with Configuration class (dotnet#55597)
  Finish migrating RSAOpenSsl from RSA* to EVP_PKEY*
  Disable generic math (dotnet#55540)
  Obsolete CryptoConfig.EncodeOID (dotnet#55592)
  Address System.Net.Http.WinHttpHandler's nullable warnings targeting .NETCoreApp (dotnet#54995)
  Enable Http2_MultipleConnectionsEnabled_ConnectionLimitNotReached_ConcurrentRequestsSuccessfullyHandled (dotnet#55572)
  Fix Task.WhenAny failure mode when passed ICollection of zero tasks (dotnet#55580)
  Consume DistributedContextPropagator in DiagnosticsHandler (dotnet#55392)
  Add property ordering feature (dotnet#55586)
  Reduce subtest count in Reflection (dotnet#55537)
  ...
alnikola added a commit that referenced this pull request Jul 14, 2021
@alnikola
Disable Http2_MultipleConnectionsEnabled_ConnectionLimitNotReached_Co…
b45b5fc 
…ncurrentRequestsSuccessfullyHandled

Reverts #55572. The test is still failing.
@karelz karelz added this to the 6.0.0 milestone Jul 15, 2021
@ManickaP ManickaP mentioned this pull request Jul 20, 2021
Merged
@ghost ghost locked as resolved and limited conversation to collaborators Aug 14, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@stephentoub stephentoub stephentoub approved these changes

Assignees
No one assigned
Labels
area-System.Net.Http
Projects
None yet
Milestone
6.0.0
4 participants
@alnikola @jeffhandley @stephentoub @karelz