TS-4087: Reduce SETTINGS_MAX_CONCURRENT_STREAMS when too many streams #485
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Contributor
|
I will take a look at this. |
| Http2ConnectionState::_adjust_concurrent_stream() | ||
| { | ||
| int64_t current_client_streams = 0; | ||
| RecGetRawStatSum(http2_rsb, HTTP2_STAT_CURRENT_CLIENT_STREAM_COUNT, ¤t_client_streams); |
Contributor
There was a problem hiding this comment.
This is getting the total stream value across all the threads and then you are comparing it to the per thread value below.
Contributor
Author
There was a problem hiding this comment.
Ah, RecGetRawStatSum gets global + thread local stats. I'll fix this. Thanks.
I thought RecGetGlobalRawStatSum is global and RecGetRawStatSum is thread local one;)
Add below variables in records.config - proxy.config.http2.min_concurrent_streams_in - proxy.config.http2.max_active_streams_in When connection wide active stream are larger than proxy.config.http2.max_active_streams_in, SETTINGS_MAX_CONCURRENT_STREAMS is reduced to proxy.config.http2.min_concurrent_streams_in. If the value of proxy.config.http2.max_active_streams_in is 0, there is no limit.
Contributor
Author
PTAL |
Contributor
|
How would this prevent a DDOS attack if clients established a bunch of connections and then made requests up to the max number of streams per connection (100)? I think it would be better to dynamically adjust the max stream when a new stream is created. This is better then nothing, which we have now, so I am OK with it. 👍 |
Contributor
Author
|
That is good point. This patch doesn't cover such cases.
I agree. I'll file a new issue on JIRA. |
SolidWallOfCode
pushed a commit
to SolidWallOfCode/trafficserver
that referenced
this pull request
Jul 23, 2021
(cherry picked from commit 8b260f1)
SolidWallOfCode
pushed a commit
to SolidWallOfCode/trafficserver
that referenced
this pull request
Jul 23, 2021
* Revert "Fix issue with plugin supplied address - suppress DNS lookup in that case. (apache#486)" This reverts commit 5023a7d. * Revert "Fix: avoid reusing null hostent if the DNS query failed. (apache#485)" This reverts commit 85be058. * Revert "Revert API change - maintain 9.x compatibility. (apache#478)" This reverts commit 26a7fc9. * Revert "Add method to write an IpAddr value to a sockaddr. (apache#7821)" This reverts commit 4a5ec19. * Revert "DNS: Clean up argument passing to DNS queries. (apache#7778)" This reverts commit bc546e4. * Revert "Add overload for memcpy to take a destination buffer and source string_view / TextView (apache#7732)" This reverts commit e782f7b. * Revert "Add basic type aliases for std::chrono types to ink_time.h for future use. (apache#7482)" This reverts commit 9554c05. * Revert "Add Au test for strategies.yaml, with consistent hashing, with fallover. (apache#7914) (apache#480)" This reverts commit 123b53f. * Revert "HostDB restructuring." This reverts commit dae938f.
SolidWallOfCode
added a commit
to SolidWallOfCode/trafficserver
that referenced
this pull request
Jul 23, 2021
* HostDB restructuring. * Add method to write an IpAddr value to a sockaddr. (apache#7821) (cherry picked from commit fbdbb5b) * DNS: Clean up argument passing to DNS queries. (apache#7778) (cherry picked from commit 6009f46) (cherry picked from commit bc546e4) * Fix: avoid reusing null hostent if the DNS query failed. (apache#485) (cherry picked from commit 8b260f1) (cherry picked from commit 85be058) * Fix brokenness in Lua do to API change and reversion. (cherry picked from commit 6266b4f) * HostDB: Fix plugin API port error. (cherry picked from commit 34518d1) * Revert API change - maintain 9.x compatibility. (cherry picked from commit 330a203) Co-authored-by: Alan M. Carroll <amc@apache.org>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Add below variables in records.config
When connection wide active streams are larger than proxy.config.http2.max_active_streams_in,
SETTINGS_MAX_CONCURRENT_STREAMS is reduced to proxy.config.http2.min_concurrent_streams_in.
TS-4087