Skip to content

Conversation

@randall
Copy link
Contributor

@randall randall commented Mar 21, 2022

No description provided.

@randall randall added the HTTP/2 label Mar 21, 2022
@randall randall added this to the 10.0.0 milestone Mar 21, 2022
@randall randall self-assigned this Mar 21, 2022
@randall randall force-pushed the adds_http2.default_buffer_water_mark branch from 658841b to 17c1396 Compare March 21, 2022 23:29
@randall
Copy link
Contributor Author

randall commented Mar 22, 2022

[approve ci fedora]

@ezelkow1
Copy link
Member

@randall I dont think its stuck, just had another fedora failure. It looks like autest is running though so let that one finish first and then can retry just fedora

@masaori335
Copy link
Contributor

@keesspoelstra is going to review this. Thanks in advance!

@apache apache deleted a comment from ezelkow1 Mar 22, 2022
@randall randall force-pushed the adds_http2.default_buffer_water_mark branch 2 times, most recently from 797c9d3 to 67bdb2d Compare March 22, 2022 18:50
@bryancall
Copy link
Contributor

I will take a look at this PR too.

payload_length = 0;
}

if (payload_length > 0 && this->session->is_write_high_water()) {
Copy link
Contributor

Choose a reason for hiding this comment

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

You are not going to send data to the client unless there is 32k to send?

Copy link
Contributor

Choose a reason for hiding this comment

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

If you're wondering about the timing of the this->session->flush(), it is called with several conditions.

  • e.g. if the response body is 10 bytes, it'll be sent to the client immediately.

This is checking the high watermark of the session buffer to stop it keep growing.

@randall randall force-pushed the adds_http2.default_buffer_water_mark branch from 67bdb2d to 0b0a988 Compare May 4, 2022 01:02
@randall randall force-pushed the adds_http2.default_buffer_water_mark branch from 0b0a988 to 5e6e160 Compare May 4, 2022 02:50
@randall randall merged commit 0f2d941 into apache:master May 4, 2022
zwoop pushed a commit that referenced this pull request May 4, 2022
Co-authored-by: Masaori Koshiba <masaori@apache.org>
(cherry picked from commit 0f2d941)
@zwoop zwoop modified the milestones: 10.0.0, 9.2.0 May 4, 2022
moonchen pushed a commit to moonchen/trafficserver that referenced this pull request May 26, 2022
* asf/9.2.x:
  Updated ChangeLog
  Add http2.default_buffer_water_mark config to tune latency (apache#8747)
  Updated ChangeLog
  Updated ChangeLog
  Revert remap.config loading changes (apache#8803)
@randall randall deleted the adds_http2.default_buffer_water_mark branch September 14, 2022 21:22
bneradt added a commit that referenced this pull request Apr 3, 2024
When I ported #8747 into HTTP/2 to origin logic, I missed setting the watermark on outgoing connections. This essentially made it so that regardless of configuration, POST requests or the like would always send every little bit of DATA frames regardless of the configured watermark. The watermark was effectively zero on the server side. This patch fixes that by appying the configured watermark.

This patch also fixes the HTTP/2 write_time_threshold configuration so it consistently wakes up ATS to transmit the frames held up behidn write_buffer_threshold. If the write_buffer_threshold is not met, then I noticed ATS never woke up to send the accumulated frames until a xmit with a flush was sent at the end of the transaction. This ensures that the frames are transmitted per the write_time_threshold by scheduling an event for the retransmission.

Here's a list of changes made in this patch:

* Adding a write_threshold test
* Set buffer watermark for origin h2 connections
* HTTP2_SESSION_EVENT_XMIT -> HTTP2_SESSION_EVENT_PRIO
* schedule flushing event for write_time_threshold
cmcfarlen pushed a commit that referenced this pull request Apr 5, 2024
When I ported #8747 into HTTP/2 to origin logic, I missed setting the watermark on outgoing connections. This essentially made it so that regardless of configuration, POST requests or the like would always send every little bit of DATA frames regardless of the configured watermark. The watermark was effectively zero on the server side. This patch fixes that by appying the configured watermark.

This patch also fixes the HTTP/2 write_time_threshold configuration so it consistently wakes up ATS to transmit the frames held up behidn write_buffer_threshold. If the write_buffer_threshold is not met, then I noticed ATS never woke up to send the accumulated frames until a xmit with a flush was sent at the end of the transaction. This ensures that the frames are transmitted per the write_time_threshold by scheduling an event for the retransmission.

Here's a list of changes made in this patch:

* Adding a write_threshold test
* Set buffer watermark for origin h2 connections
* HTTP2_SESSION_EVENT_XMIT -> HTTP2_SESSION_EVENT_PRIO
* schedule flushing event for write_time_threshold

(cherry picked from commit 66940f5)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants