gerlowskija opened a new pull request, #2621:
URL: https://github.com/apache/solr/pull/2621

   https://issues.apache.org/jira/browse/SOLR-17394
   
   # Description
   
   Prior to this commit, IndexFetcher would mis-read 404 responses from other 
nodes.  It eventually catches this mistake and retries, but not before it 
wastefully allocates a massive (> 1GB) byte array.
   
   These allocations were relatively infrequent, as they require out of date 
leader information to occur.  But they are big enough to impact performance in 
production, and were observed to cause OOM's when running tests.
   
   # Solution
   
   This commit avoids this by checking the HTTP status code of all IndexFetcher 
requests/responses, even those that utilize InputStreamResponseParser.
   
   # Tests
   
   I don't have unit tests for this as it's a bit hard to exercise in 
isolation.  But I am able to see the newly-introduced IOE message in the logs, 
and the massive `byte[]` is no longer allocated.
   
   The other way I've tested this, such as it is, is through 'beasting' a 
particular test which trigger a OOM and heap dump on about every 1/10 
iterations.  Setting `tests.heapsize` to any moderate value (e.g. `1GB`) in 
`gradle.properties` and running the command below reproduces the OOM reliably 
for me on `main` without this PR.  (And reproduces the OOM never with the PR in 
place.)
   
   ```
   ./gradlew -p solr/solrj beast -Ptests.seed=F024B5067D7FCE8F 
-Ptests.neverUpToDate=true -Ptests.dups=50 --tests CloudHttp2SolrClientTest
   ```
   
   # Checklist
   
   Please review the following and check all that apply:
   
   - [x] I have reviewed the guidelines for [How to 
Contribute](https://github.com/apache/solr/blob/main/CONTRIBUTING.md) and my 
code conforms to the standards described there to the best of my ability.
   - [x] I have created a Jira issue and added the issue ID to my pull request 
title.
   - [x] I have given Solr maintainers 
[access](https://help.github.com/en/articles/allowing-changes-to-a-pull-request-branch-created-from-a-fork)
 to contribute to my PR branch. (optional but recommended)
   - [x] I have developed this patch against the `main` branch.
   - [x] I have run `./gradlew check`.
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to