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

http.sys (6.0 backport): new option for response buffering #48072

Merged
merged 2 commits into from
May 8, 2023

Conversation

mgravell
Copy link
Member

@mgravell mgravell commented May 4, 2023

HttpSys: new option for response buffering

8.0 feature: #47776

(note: this is a cherry-pick from the internal experimental/validation environment)

Description

As per old PR 418 (direct copy), enables kernel mode response-buffering in http.sys - with modification to use app-context switch rather than public API change

Customer Impact

Without this buffer, writes are sent as-written direct thru http.sys; this has two problems:

  • fragmentation if writes are small
  • issues with latency

Regression?

  • Yes
  • No

[If yes, specify the version the behavior has regressed from]

Risk

  • High
  • Medium
  • Low - opt in, no impact to consumers not using this flag

[Justify the selection above]

Verification

  • Manual (required) - will try to verify with internal MSFT consumer (targeting net6.0)
  • Automated

Packaging changes reviewed?

  • Yes
  • No
  • N/A

When servicing release/2.1

  • Make necessary changes in eng/PatchConfig.props

# {PR title}

HttpSys: new option for response buffering

## Description

{Detail}

As per [old PR 418](aspnet/HttpSysServer#418) (direct copy), enables kernel mode response-buffering in http.sys - with modification to use app-context switch rather than public API change

## Customer Impact

Without this buffer, writes are sent as-written direct thru http.sys; this has two problems:

- fragmentation if writes are small
- issues with latency

## Regression?

- [ ] Yes
- [x] No

[If yes, specify the version the behavior has regressed from]

## Risk

- [ ] High
- [ ] Medium
- [x] Low - opt in, no impact to consumers not using this flag

[Justify the selection above]

## Verification

- [x] Manual (required) - will try to verify with internal MSFT consumer (targeting net6.0)
- [ ] Automated

## Packaging changes reviewed?

- [ ] Yes
- [ ] No
- [x] N/A

----

## When servicing release/2.1

- [ ] Make necessary changes in eng/PatchConfig.props
@ghost ghost added the area-runtime label May 4, 2023
@ghost ghost added this to the 6.0.x milestone May 4, 2023
@ghost
Copy link

ghost commented May 4, 2023

Hi @mgravell. If this is not a tell-mode PR, please make sure to follow the instructions laid out in the servicing process document.
Otherwise, please add tell-mode label.

Co-authored-by: Aditya Mandaleeka <adityamandaleeka@users.noreply.github.com>
@mgravell
Copy link
Member Author

mgravell commented May 5, 2023

@wtgodbe for servicing when possible

@adityamandaleeka
Copy link
Member

Tactics mail sent, awaiting approval.

@adityamandaleeka
Copy link
Member

Approved.

@adityamandaleeka adityamandaleeka added the Servicing-approved Shiproom has approved the issue label May 6, 2023
@ghost
Copy link

ghost commented May 6, 2023

Hi @mgravell. This PR was just approved to be included in the upcoming servicing release. Somebody from the @dotnet/aspnet-build team will get it merged when the branches are open. Until then, please make sure all the CI checks pass and the PR is reviewed.

@wtgodbe
Copy link
Member

wtgodbe commented May 8, 2023

Components failure is known/unrelated

@wtgodbe wtgodbe merged commit 832ccdc into release/6.0 May 8, 2023
@wtgodbe wtgodbe deleted the marc/r6-kernel-buffer branch May 8, 2023 18:44
@ghost ghost modified the milestones: 6.0.x, 6.0.18 May 8, 2023
@ghost ghost added the area-infrastructure Includes: MSBuild projects/targets, build scripts, CI, Installers and shared framework label May 8, 2023
@amcasey amcasey added area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions and removed area-runtime labels Jun 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-infrastructure Includes: MSBuild projects/targets, build scripts, CI, Installers and shared framework area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions Servicing-approved Shiproom has approved the issue
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants