Cap testLimitsInFlightRequests at the Netty pipeline depth of 128 - #22636
Open
serhiy-bzhezytskyy wants to merge 1 commit into
Open
Conversation
Netty 4.2.16 added a maxPipelineDepth to HttpContentEncoder with a default of 128 (netty/netty#17063), so a single connection carrying more pipelined requests than that gets an IllegalStateException("maxPipelineDepth exceeded: 128") and the server closes it. The client then receives none of its responses and fails on the 30s latch: "Failed to get all expected responses: 150 left". The Netty upgrade (opensearch-project#22403) already applied this cap to testDoesNotLimitExcludedRequests in this same class, going from 1500 to 128 with a comment pointing at netty/netty#17068. testLimitsInFlightRequests was left at 150 and hits the same wall, which is what makes this class keep showing up in flaky reports (opensearch-project#18875) with a failure that is not actually random: the encoder queue drains as responses are written, so the limit is only reached when requests outrun responses -- which happens on a loaded CI host and not on a fast local one. 128 still exercises what the test is for: the in-flight-requests breaker is set to 2 KB and trips at 2880 bytes, i.e. after a handful of requests. Verified by mutation -- raising the breaker to 2 GB fails the test, so the TOO_MANY_REQUESTS assertion still bites. Note that the configurable variant reta added in netty/netty#17068 is not available yet: it merged on 2026-07-10, three days after 4.2.16.Final was released, and 4.2.17 is not out. Once it is, this cap can be driven from http.pipelining.max_events instead. Signed-off-by: serhiy-bzhezytskyy <me@serhiy-bzhezytskyy.com>
Contributor
PR Reviewer Guide 🔍Here are some key observations to aid the review process:
|
Contributor
|
❕ Gradle check result for df4c3c8: UNSTABLE Please review all flaky tests that succeeded after retry and create an issue if one does not already exist to track the flaky failure. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #22636 +/- ##
============================================
+ Coverage 71.39% 71.48% +0.08%
- Complexity 76808 76883 +75
============================================
Files 6148 6148
Lines 357994 357999 +5
Branches 52179 52179
============================================
+ Hits 255607 255929 +322
+ Misses 82054 81735 -319
- Partials 20333 20335 +2 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
This was referenced Aug 3, 2026
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Description
Netty 4.2.16 added a
maxPipelineDepthtoHttpContentEncoderwith a default of 128 (netty/netty#17063). A single connection carrying more pipelined requests than that getsIllegalStateException: maxPipelineDepth exceeded: 128, the server closes the connection, and the client receives none of its responses — failing on the 30s latch withFailed to get all expected responses: 150 left.The Netty upgrade in #22403 already applied this cap to
testDoesNotLimitExcludedRequestsin this same class (1500 → 128, with a comment pointing at netty/netty#17068).testLimitsInFlightRequestswas left at 150 and hits the same wall.That is why this class keeps appearing in flaky reports (#18875) with a failure that is not random: the encoder queue drains as responses are written, so the limit is only reached when requests outrun responses — which happens on a loaded CI host and not on a fast local one. The autocut report attributes the class to cluster-state timeouts; this failure mode is different, and its stack trace shows
maxPipelineDepth exceeded: 128instead.128 still exercises what the test is for: the in-flight-requests breaker is set to 2 KB and trips at 2880 bytes, i.e. after a handful of requests. Mutation-checked — raising the breaker to 2 GB fails the test, so the
TOO_MANY_REQUESTSassertion still bites.The configurable variant added in netty/netty#17068 is not available yet: it merged on 2026-07-10, three days after 4.2.16.Final was released, and there is no 4.2.17. Once there is, this cap can be driven from
http.pipelining.max_eventsinstead.Related Issues
Relates to #18875, #22403
Check List
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.