Skip to content

Use getResponseBodyAsStream instead of getResponseBody#11465

Merged
xiangfu0 merged 1 commit intoapache:masterfrom
soumitra-st:get-response-stream
Aug 31, 2023
Merged

Use getResponseBodyAsStream instead of getResponseBody#11465
xiangfu0 merged 1 commit intoapache:masterfrom
soumitra-st:get-response-stream

Conversation

@soumitra-st
Copy link
Contributor

While running a large test, below log is seen often.

2023/08/25 03:33:51.864 WARN [HttpMethodBase] [pool-12-thread-5] Going to buffer response body of large or unknown size. Using getResponseBodyAsStream instead is recommended.

This is coming from Apache library when the buffered response size is larger than http.method.response.buffer.warnlimit.

This PR replaces few usages of getResponseBody with getResponseBodyAsStream

…verted into JSON. Changing that to directly populate JSON using the Stream to reduce the memory overhead.
@codecov-commenter
Copy link

codecov-commenter commented Aug 31, 2023

Codecov Report

Attention: Patch coverage is 0% with 17 lines in your changes missing coverage. Please review.

Project coverage is 62.96%. Comparing base (f14700a) to head (9d00bf4).
Report is 2929 commits behind head on master.

Files with missing lines Patch % Lines
...oller/response/ControllerTenantBrokerResponse.java 0.00% 4 Missing ⚠️
...inot/client/controller/response/TableResponse.java 0.00% 4 Missing ⚠️
...not/client/controller/response/SchemaResponse.java 0.00% 3 Missing ⚠️
...inot/client/JsonAsyncHttpPinotClientTransport.java 0.00% 2 Missing ⚠️
.../controller/response/ControllerResponseFuture.java 0.00% 2 Missing ⚠️
...main/java/org/apache/pinot/client/BrokerCache.java 0.00% 1 Missing ⚠️
...ain/java/org/apache/pinot/spi/utils/JsonUtils.java 0.00% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##             master   #11465      +/-   ##
============================================
+ Coverage     62.92%   62.96%   +0.04%     
- Complexity     1108     1110       +2     
============================================
  Files          2318     2318              
  Lines        124328   124321       -7     
  Branches      18980    18980              
============================================
+ Hits          78234    78280      +46     
+ Misses        40539    40482      -57     
- Partials       5555     5559       +4     
Flag Coverage Δ
integration <0.01% <0.00%> (ø)
integration1 <0.01% <0.00%> (ø)
integration2 0.00% <0.00%> (ø)
java-11 62.89% <0.00%> (-0.02%) ⬇️
java-17 62.80% <0.00%> (+0.02%) ⬆️
java-20 62.83% <0.00%> (+0.05%) ⬆️
temurin 62.96% <0.00%> (+0.04%) ⬆️
unittests 62.96% <0.00%> (+0.04%) ⬆️
unittests1 67.45% <0.00%> (+0.02%) ⬆️
unittests2 14.46% <0.00%> (+0.02%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@xiangfu0 xiangfu0 merged commit b3b0b9e into apache:master Aug 31, 2023
KKcorps pushed a commit to KKcorps/incubator-pinot that referenced this pull request Sep 5, 2023
…verted into JSON. Changing that to directly populate JSON using the Stream to reduce the memory overhead. (apache#11465)
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.

3 participants