Skip to content

[6.0] Allow zero byte reads on the test server response body #44020

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

Merged

Conversation

Tratcher
Copy link
Member

@Tratcher Tratcher commented Sep 15, 2022

YARP adopted the zero-byte-read pattern to reduce memory usage on long lived connections. However, we've found several existing streams in the framework that do not support that pattern.

Description

In-memory web app testing using TestServer is a pattern we recommend to make tests more reliable. Customers can set up YARP to proxy to an in-memory TestServer instance to test multiple applications together. However, when YARP adopted the zero-byte-read pattern, TestServer didn't support that and the scenario broke.

There is no known workaround for 6.0. The issue was partially fixed in 7.0 via new Stream Memory overloads.

Fixes #41692

Customer Impact

YARP customers are currently blocked from testing by proxying to in-memory apps via TestServer.

Regression?

  • Yes
  • No

This worked with YARP 1.0 before zero-byte-reads were adopted in 1.1.

Risk

  • High
  • Medium
  • Low

This is a testing component.

Verification

  • Manual (required)
  • Automated

Packaging changes reviewed?

  • Yes
  • No
  • N/A

@Tratcher Tratcher self-assigned this Sep 15, 2022
@ghost ghost added the area-runtime label Sep 15, 2022
@ghost ghost added this to the 6.0.x milestone Sep 15, 2022
@ghost
Copy link

ghost commented Sep 15, 2022

Hi @Tratcher. 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.

@Tratcher Tratcher added the Servicing-consider Shiproom approval is required for the issue label Sep 15, 2022
@ghost
Copy link

ghost commented Sep 15, 2022

Hi @Tratcher. Please make sure you've updated the PR description to use the Shiproom Template. Also, make sure this PR is not marked as a draft and is ready-to-merge.

To learn more about how to prepare a servicing PR click here.

Copy link
Member

@wtgodbe wtgodbe left a comment

Choose a reason for hiding this comment

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

Seems reasonable to me

@Tratcher Tratcher changed the title Allow zero byte reads on the test server response body [6.0] Allow zero byte reads on the test server response body Sep 20, 2022
@Tratcher Tratcher marked this pull request as ready for review September 20, 2022 17:15
@Tratcher Tratcher added Servicing-approved Shiproom has approved the issue and removed Servicing-consider Shiproom approval is required for the issue labels Sep 21, 2022
@wtgodbe wtgodbe modified the milestones: 6.0.x, 6.0.11 Oct 4, 2022
@wtgodbe wtgodbe merged commit 1c61e0c into dotnet:release/6.0 Oct 4, 2022
@Tratcher Tratcher deleted the tratcher/release/6.0/testresponsezbr branch October 4, 2022 20:12
@amcasey amcasey added area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions and removed area-runtime labels Aug 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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