Skip to content

Simplify the parameters for DistinctCountSmartHLLAggregationFunction #8566

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
siddharthteotia merged 1 commit into from
Apr 20, 2022
Merged

Simplify the parameters for DistinctCountSmartHLLAggregationFunction #8566

siddharthteotia merged 1 commit into from
Apr 20, 2022

Conversation

@Jackie-Jiang
Copy link
Contributor

@Jackie-Jiang Jackie-Jiang commented Apr 19, 2022 •
edited
Loading

Currently the parameters for the DistinctCountSmartHLLAggregationFunction is too long and can be hard to use. Since they are always under the scope of the function, we may simplify them for usability. The old parameter keys are still supported.

Release Notes

Change the following parameter keys for DISTINCT_COUNT_SMART_HLL aggregation function:

  • hllConversionThreshold -> threshold
  • hllLog2m ->log2m

@Jackie-Jiang Jackie-Jiang added the release-notes Referenced by PRs that need attention when compiling the next release notes label Apr 19, 2022
@codecov-commenter
Copy link

codecov-commenter commented Apr 19, 2022 •
edited
Loading

Codecov Report

Merging #8566 (1b73849) into master (829ede9) will decrease coverage by 34.10%.
The diff coverage is 8.51%.

❗ Current head 1b73849 differs from pull request most recent head f018cc9. Consider uploading reports for the commit f018cc9 to get more accurate results

@@              Coverage Diff              @@
##             master    #8566       +/-   ##
=============================================
- Coverage     70.83%   36.72%   -34.11%     
+ Complexity     4291       84     -4207     
=============================================
  Files          1686     1687        +1     
  Lines         88226    88248       +22     
  Branches      13351    13352        +1     
=============================================
- Hits          62492    32407    -30085     
- Misses        21362    53216    +31854     
+ Partials       4372     2625     -1747
Flag Coverage Δ
integration1 27.30% <8.51%> (-0.13%) ⬇️
integration2 26.02% <8.51%> (+0.09%) ⬆️
unittests1 ?
unittests2 14.12% <6.38%> (-0.01%) ⬇️

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

Impacted Files Coverage Δ
...troller/api/exception/NoTaskMetadataException.java 0.00% <0.00%> (ø)
...roller/api/resources/PinotTaskRestletResource.java 4.05% <0.00%> (-0.27%) ⬇️
...perator/query/NonScanBasedAggregationOperator.java 53.76% <0.00%> (-27.96%) ⬇️
...tion/DistinctCountSmartHLLAggregationFunction.java 0.00% <0.00%> (-11.14%) ⬇️
...lix/core/minion/PinotHelixTaskResourceManager.java 40.57% <25.00%> (-0.55%) ⬇️
...apache/pinot/controller/BaseControllerStarter.java 82.64% <100.00%> (ø)
.../java/org/apache/pinot/spi/utils/BooleanUtils.java 0.00% <0.00%> (-100.00%) ⬇️
...java/org/apache/pinot/spi/trace/BaseRecording.java 0.00% <0.00%> (-100.00%) ⬇️
...java/org/apache/pinot/spi/trace/NoOpRecording.java 0.00% <0.00%> (-100.00%) ⬇️
...ava/org/apache/pinot/spi/config/table/FSTType.java 0.00% <0.00%> (-100.00%) ⬇️
... and 970 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 829ede9...f018cc9. Read the comment docs.

@siddharthteotia siddharthteotia merged commit 5687f27 into apache:master Apr 20, 2022
@Jackie-Jiang Jackie-Jiang deleted the smart_hll_parameters branch April 20, 2022 17:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@xiangfu0 xiangfu0 xiangfu0 approved these changes

@siddharthteotia siddharthteotia siddharthteotia approved these changes

Assignees

No one assigned

Labels

release-notes Referenced by PRs that need attention when compiling the next release notes

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@Jackie-Jiang @codecov-commenter @xiangfu0 @siddharthteotia