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

[UNDERTOW-1818] AbstractServletInputStreamTestCase.runTestParallel fails with bytes out of order #1384

Merged
merged 1 commit into from Oct 11, 2022

Conversation

rmartinc
Copy link
Contributor

Issue: https://issues.redhat.com/browse/UNDERTOW-1818

There are different changes in the PR although the former is the main one (the rest fixes minor issues in the same AbstractServletInputStreamTestCase test):

  1. The direct solution for the issue is in the Connectors class. As the method is called ungetRequestBytes the order is directly changed. If we identify that something needs to insert real new data at the current point in the exchange (not data that was read before and is push back to be read again) another method can be created with the previous order. I think that all the callers are using the re-read / push-back idea.
  2. Adding a max queue size because sometimes proxy tests fail (error 500) when concurrency tests start in input stream classes. The proxy conf now allows the requests to be queued up to 20 (max concurrency).
  3. Setting a timeout of 60s for all the proxy configurations and inside the async stream tests. Previously it was 30 and 120 (although servlets had default 30s), so aligning everything to 60s. It was enough for me.
  4. Reducing the concurrency of the input stream tests to 4 * CPUs up to previous 20 (maximum). Envs with only a few CPUs are limited to cope with the requests in 60s.

In my envs (windows-2016 and linux-fedora) the ServletInputStreamRequestBufferingTestCase was executed with -P proxy option 50 times without issues. Let's see how the tests go.

@fl4via fl4via added bug fix Contains bug fix(es) next release This PR will be merged before next release or has already been merged (for payload double check) waiting CI check Ready to be merged but waiting for CI check and removed waiting CI check Ready to be merged but waiting for CI check labels Oct 11, 2022
@fl4via fl4via merged commit bd9034e into undertow-io:master Oct 11, 2022
@fl4via fl4via removed the next release This PR will be merged before next release or has already been merged (for payload double check) label Oct 12, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug fix Contains bug fix(es)
Projects
None yet
2 participants