Skip to content

Add test coverage for consuming segment using on-heap memory. #8385

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.

Sign up for GitHub

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

Jump to bottom
Merged
Jackie-Jiang merged 1 commit into from
Mar 22, 2022
Merged

Add test coverage for consuming segment using on-heap memory. #8385

Jackie-Jiang merged 1 commit into from
Mar 22, 2022
+12 −6

Conversation

@mayankshriv
Copy link
Contributor

@mayankshriv mayankshriv commented Mar 22, 2022

Description

With PR #8380, we modified the default value for
pinot.server.instance.realtime.alloc.offheap to use off-heap. Explicitly setting
it to use on-heap in some of the integration tests for coverage.

Upgrade Notes

Does this PR prevent a zero down-time upgrade? (Assume upgrade order: Controller, Broker, Server, Minion)

  • Yes (Please label as backward-incompat, and complete the section below on Release Notes)

Does this PR fix a zero-downtime upgrade introduced earlier?

  • Yes (Please label this as backward-incompat, and complete the section below on Release Notes)

Does this PR otherwise need attention when creating release notes? Things to consider:

  • New configuration options
  • Deprecation of configurations
  • Signature changes to public methods/interfaces
  • New plugins added or old plugins removed
  • Yes (Please label this PR as release-notes and complete the section on Release Notes)

Release Notes

Documentation

@mayankshriv mayankshriv mentioned this pull request Mar 22, 2022
Merged
3 tasks
@mcvsubbu
Copy link
Contributor

mcvsubbu commented Mar 22, 2022

Can you add the appropriate labels for release notes please? thanks

@mayankshriv
Copy link
Contributor Author

mayankshriv commented Mar 22, 2022

Can you add the appropriate labels for release notes please? thanks

The original PR that did the config value change has all the labels and release notes. This one is just making sure we have coverage for non-default.

@codecov-commenter
Copy link

codecov-commenter commented Mar 22, 2022

Codecov Report

Merging #8385 (00239b0) into master (2d809ff) will decrease coverage by 31.96%.
The diff coverage is 70.96%.

❗ Current head 00239b0 differs from pull request most recent head 83f4b77. Consider uploading reports for the commit 83f4b77 to get more accurate results

@@              Coverage Diff              @@
##             master    #8385       +/-   ##
=============================================
- Coverage     69.75%   37.78%   -31.97%     
+ Complexity     4212       84     -4128     
=============================================
  Files          1653     1654        +1     
  Lines         86615    86452      -163     
  Branches      13080    13037       -43     
=============================================
- Hits          60420    32668    -27752     
- Misses        21966    51185    +29219     
+ Partials       4229     2599     -1630
Flag Coverage Δ
integration1 28.68% <13.97%> (+0.13%) ⬆️
integration2 27.15% <53.76%> (?)
unittests1 ?
unittests2 14.13% <17.39%> (+0.05%) ⬆️

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

Impacted Files Coverage Δ
.../pinot/core/query/reduce/ResultReducerFactory.java 78.57% <ø> (+3.57%) ⬆️
.../java/org/apache/pinot/core/util/GapfillUtils.java 6.17% <0.00%> (-68.02%) ⬇️
...transform/function/IsNotNullTransformFunction.java 60.71% <60.71%> (ø)
...or/transform/function/IsNullTransformFunction.java 67.85% <67.85%> (ø)
...roker/routing/segmentpruner/TimeSegmentPruner.java 80.22% <75.00%> (-0.66%) ⬇️
...r/transform/function/TransformFunctionFactory.java 81.81% <80.00%> (-1.82%) ⬇️
...e/pinot/common/function/TransformFunctionType.java 84.52% <100.00%> (-15.48%) ⬇️
.../pinot/common/function/scalar/StringFunctions.java 18.46% <100.00%> (-54.56%) ⬇️
...ller/api/resources/PinotSchemaRestletResource.java 63.39% <100.00%> (+0.66%) ⬆️
...not/core/operator/transform/TransformOperator.java 83.33% <100.00%> (-10.22%) ⬇️
... and 993 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2d809ff...83f4b77. Read the comment docs.

@mayankshriv
Add test coverage for consuming segment using on-heap.
c6c8287 
With PR apache#8380, we modified the default value for
`pinot.server.instance.realtime.alloc.offheap` to use off-heap. Explicitly setting
it to use on-heap in some of the integration tests for coverage.
@Jackie-Jiang Jackie-Jiang merged commit 3a86e3d into apache:master Mar 22, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Jackie-Jiang Jackie-Jiang Jackie-Jiang approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@mayankshriv @mcvsubbu @codecov-commenter @Jackie-Jiang