-
Notifications
You must be signed in to change notification settings - Fork 7.6k
Support HTTP persistent connections in Web Cmdlets #19249
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
Support HTTP persistent connections in Web Cmdlets #19249
Conversation
...t.PowerShell.Commands.Utility/commands/utility/WebCmdlet/Common/WebRequestPSCmdlet.Common.cs
Outdated
Show resolved
Hide resolved
...t.PowerShell.Commands.Utility/commands/utility/WebCmdlet/Common/WebRequestPSCmdlet.Common.cs
Outdated
Show resolved
Hide resolved
...t.PowerShell.Commands.Utility/commands/utility/WebCmdlet/Common/WebRequestPSCmdlet.Common.cs
Show resolved
Hide resolved
...t.PowerShell.Commands.Utility/commands/utility/WebCmdlet/Common/WebRequestPSCmdlet.Common.cs
Show resolved
Hide resolved
src/Microsoft.PowerShell.Commands.Utility/commands/utility/WebCmdlet/WebRequestSession.cs
Show resolved
Hide resolved
test/powershell/Modules/Microsoft.PowerShell.Utility/WebCmdlets.Tests.ps1
Outdated
Show resolved
Hide resolved
test/powershell/Modules/Microsoft.PowerShell.Utility/WebCmdlets.Tests.ps1
Outdated
Show resolved
Hide resolved
test/powershell/Modules/Microsoft.PowerShell.Utility/WebCmdlets.Tests.ps1
Outdated
Show resolved
Hide resolved
test/powershell/Modules/Microsoft.PowerShell.Utility/WebCmdlets.Tests.ps1
Outdated
Show resolved
Hide resolved
test/powershell/Modules/Microsoft.PowerShell.Utility/WebCmdlets.Tests.ps1
Outdated
Show resolved
Hide resolved
Codefactor is complaining about some tests that were not added by this PR using plain text as a password. @CarloToso will that impact being able to merge this PR? |
@stevenebutler Don't worry about those two codefactor issues, I introduced the in #19190 |
src/Microsoft.PowerShell.Commands.Utility/commands/utility/WebCmdlet/WebRequestSession.cs
Show resolved
Hide resolved
test/powershell/Modules/Microsoft.PowerShell.Utility/WebCmdlets.Tests.ps1
Outdated
Show resolved
Hide resolved
test/powershell/Modules/Microsoft.PowerShell.Utility/WebCmdlets.Tests.ps1
Outdated
Show resolved
Hide resolved
test/powershell/Modules/Microsoft.PowerShell.Utility/WebCmdlets.Tests.ps1
Outdated
Show resolved
Hide resolved
Use persistent HTTP connections when Invoke-WebRequest uses the -Session variable. This is achieved by * tracking the HttpClient object in the saved WebRequestSession * monitoring properties in WebRequestSession and rebuilding the HttpClient if they are changed. This is done to ensure that existing scripts that might change parameters between invocations of IWR actually apply the changes so they will behave in a backwards compatible manner. When anything that impacts the HttpClient is changed, the persistent connection is abandoned by disposing the HttpClient and underlying HttpClientHandler and constructing a new one. A verbose message is logged when this occurs as it has a performance impact (though no worse than the existing behaviour). * To support the point 2 above, many switches were added as internal set-only properties to the WebRequestSession so changes between invocations can be detected and the handler rebuilt appropriately.
e7d0b02
to
2f21034
Compare
This PR has Quantification details
Why proper sizing of changes matters
Optimal pull request sizes drive a better predictable PR flow as they strike a
What can I do to optimize my changes
How to interpret the change counts in git diff output
Was this comment helpful? 👍 :ok_hand: :thumbsdown: (Email) |
I have squashed all commits into a single commit. |
@stevenebutler Thank you for your contribution! |
I'm happy to see this merged. You can safely close #12764 now. |
🎉 Handy links: |
I tested Preview 2 and back to back invocations of Invoke-WebRequest and Invoke-RestMethod are not re-using ports, new ones are created. |
@tonyg00 did you use the |
I did not, I just executed, Invoke-WebRequest -uri which is the most common use case for me of these cmdlets. |
Don't worry, it's a new feature, it's not yet documented. Invoke-WebRequest -Uri -SessionVariable session
Invoke-WebRequest -Uri -WebSession $session |
That is working, Thanks!! |
PR Summary
Fis #12764
This PR addresses 12764 by using persistent HTTP connections when Invoke-WebRequest uses the -Session variable. It does this by
PR Context
This PR makes a considerable difference to performance when using Invoke-WebRequest or Invoke-RestMethod with sessions when multiple requests are made to the same host. This is particularly evident for HTTPS requests, as each HTTPS connection can take hundreds of milliseconds to establish.
This PR as written passes existing tests in
WebCmdlets.Tests.ps1
and additional WebSession tests that verify the HttpClient is rebuilt when appropriate.The PR provides as much backwards compatibility as possible at the expense of the complexity of tracking how the HttpClient used in the WebSession was constructed and recreating the HttpClient in the case it would have been constructed differently between invocations in scripts that assume it is okay to change parameters between invocations with the same WebSession.
This PR squashes the commits in the original PR#19173 following multiple round of reviews by @iSazonov and @CarloToso on a previous version of the PR. PR#19173 has been withdrawn due to number of commits and volume of review comments.
Performance Impact
Performance impact of maintaining HTTP connection persistence is measured by comparing PowerShell 7.3.3 with this PR using this simple script:
PowerShell 7.3.3
PR Branch
PR Checklist
.h
,.cpp
,.cs
,.ps1
and.psm1
files have the correct copyright headerWIP:
or[ WIP ]
to the beginning of the title (theWIP
bot will keep its status check atPending
while the prefix is present) and remove the prefix when the PR is ready.(which runs in a different PS Host).