add a experiment API for upsert heap memory estimation #8355

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 7 commits into apache:master from deemoliu:qiaochu/capacity-estimate-api

May 4, 2022

Contributor

deemoliu commented Mar 15, 2022 •

edited

Loading

Description

Added a new rest resource and an endpoint for estimating heap memory usage for Pinot upsert table.
Related Pinot issue: #6729

Release Notes

Added a http endpoint for estimate the heap usage for Pinot upsert table per partition based on PK cardinality.

POST /upsert/estimateHeapUsage

Documentation

chenboat reviewed

View reviewed changes

...main/java/org/apache/pinot/controller/api/resources/PinotTableExperimentRestletResource.java Outdated

Contributor

chenboat Mar 15, 2022

This is resource is about UpsertTable only. Why not name it explicitly so that it is clear that this is about UpsertTable resource estimation?

deemoliu force-pushed the qiaochu/capacity-estimate-api branch from 68769c6 to 3bc1c66 Compare

March 29, 2022 20:52

codecov-commenter commented Mar 29, 2022 •

edited

Loading

Codecov Report

Merging #8355 (795fa15) into master (b5690a7) will decrease coverage by 0.19%.
The diff coverage is 41.66%.

@@             Coverage Diff              @@
##             master    #8355      +/-   ##
============================================
- Coverage     70.47%   70.28%   -0.20%     
- Complexity     4313     4409      +96     
============================================
  Files          1703     1718      +15     
  Lines         89569    90597    +1028     
  Branches      13564    13713     +149     
============================================
+ Hits          63123    63673     +550     
- Misses        21990    22431     +441     
- Partials       4456     4493      +37

Flag	Coverage Δ
integration1	`26.81% <4.16%> (-0.36%)`	⬇️
integration2	`25.45% <4.16%> (-0.06%)`	⬇️
unittests1	`66.42% <0.00%> (-0.03%)`	⬇️
unittests2	`13.98% <41.66%> (-0.09%)`	⬇️

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

Impacted Files	Coverage Δ
...ache/pinot/controller/api/resources/Constants.java	`23.07% <ø> (ø)`
...org/apache/pinot/spi/config/table/ColumnStats.java	`0.00% <0.00%> (ø)`
...spi/utils/builder/ControllerRequestURLBuilder.java	`0.00% <0.00%> (ø)`
...ller/api/resources/PinotUpsertRestletResource.java	`56.81% <56.81%> (ø)`
...inot/controller/helix/ControllerRequestClient.java	`63.00% <100.00%> (ø)`
...data/manager/realtime/DefaultSegmentCommitter.java	`0.00% <0.00%> (-80.00%)`	⬇️
...ot/segment/local/segment/store/TextIndexUtils.java	`76.47% <0.00%> (-23.53%)`	⬇️
...er/api/resources/LLCSegmentCompletionHandlers.java	`43.56% <0.00%> (-18.82%)`	⬇️
...ava/org/apache/pinot/client/ConnectionFactory.java	`54.54% <0.00%> (-16.05%)`	⬇️
...data/manager/realtime/SegmentCommitterFactory.java	`88.23% <0.00%> (-11.77%)`	⬇️
... and 105 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 b5690a7...795fa15. Read the comment docs.

deemoliu changed the title ~~add a experiment API for upsert heap memory estimation~~ [WIP] add a experiment API for upsert heap memory estimation

Jackie-Jiang reviewed

View reviewed changes

pinot-spi/src/main/java/org/apache/pinot/spi/config/table/ColumnStats.java Outdated Show resolved Hide resolved

.../org/apache/pinot/controller/api/resources/PinotUpsertCapacityEstimationRestletResource.java Outdated

Contributor

Jackie-Jiang Apr 5, 2022

This API is built as a tool instead of a regular restlet API as it doesn't rely on the information stored in the cluster. We should at least read the instance config info, calculate the partitions on each server, and estimate the heap usage per server

Contributor Author

deemoliu Apr 22, 2022

gotcha, I added numPartitions, replicasPerPartition and memoryPerHost

.../org/apache/pinot/controller/api/resources/PinotUpsertCapacityEstimationRestletResource.java Outdated Show resolved Hide resolved

.../org/apache/pinot/controller/api/resources/PinotUpsertCapacityEstimationRestletResource.java Outdated

Contributor

Jackie-Jiang Apr 5, 2022

If comparison column is fixed width, no need to provide it in columnStats

Contributor Author

deemoliu Apr 22, 2022 •

edited

Loading

gotcha, thanks Jackie. I will infer the comparison size from Datatype.
FieldSpec.DataType dt = schema.getFieldSpecFor(comparisonColumn).getDataType(); bytesPerValue = 52 + columnStats.getComparisonColSize();

for datatype that are not fixed width, i will throw an controllerException since it's a not supported type for comparison column e.g. String

.../org/apache/pinot/controller/api/resources/PinotUpsertCapacityEstimationRestletResource.java Outdated

Contributor

Jackie-Jiang Apr 5, 2022

We should probably also count the array size?

Contributor Author

deemoliu Apr 22, 2022

gotcha, thanks Jackie. Java has a 24 bytes array overhead and there's also 8 bytes for the actual array object, so i added 32 bytes in bytes per key.

deemoliu force-pushed the qiaochu/capacity-estimate-api branch from 0e29e7e to b38c607 Compare

April 20, 2022 16:28

deemoliu changed the title ~~[WIP] add a experiment API for upsert heap memory estimation~~ add a experiment API for upsert heap memory estimation

deemoliu force-pushed the qiaochu/capacity-estimate-api branch 8 times, most recently from ad1c735 to 210e036 Compare

April 27, 2022 04:14

Jackie-Jiang reviewed

View reviewed changes

pinot-controller/src/test/java/org/apache/pinot/controller/ControllerTestUtils.java Outdated Show resolved Hide resolved

.../org/apache/pinot/controller/api/resources/PinotUpsertCapacityEstimationRestletResource.java Outdated Show resolved Hide resolved

.../org/apache/pinot/controller/api/resources/PinotUpsertCapacityEstimationRestletResource.java Outdated Show resolved Hide resolved

.../org/apache/pinot/controller/api/resources/PinotUpsertCapacityEstimationRestletResource.java Outdated Show resolved Hide resolved

.../org/apache/pinot/controller/api/resources/PinotUpsertCapacityEstimationRestletResource.java Outdated Show resolved Hide resolved

.../org/apache/pinot/controller/api/resources/PinotUpsertCapacityEstimationRestletResource.java Outdated

Contributor

Jackie-Jiang Apr 27, 2022

We don't need replicasPerPartition because multiple replicas won't be assigned to the same host

Contributor Author

deemoliu Apr 28, 2022

gotcha.

.../org/apache/pinot/controller/api/resources/PinotUpsertCapacityEstimationRestletResource.java Outdated

Contributor

Jackie-Jiang Apr 27, 2022

totalSpacePerPartition(bytes)?

Contributor Author

deemoliu Apr 28, 2022

gotcha, thanks for suggestion.

.../org/apache/pinot/controller/api/resources/PinotUpsertCapacityEstimationRestletResource.java Outdated

Contributor

Jackie-Jiang Apr 27, 2022

(optional) Here we only calculate the map content size. Should we also consider the map entry size and the array size within the map?

Contributor Author

deemoliu Apr 28, 2022

The object overhead will vary between different machines.

I found this https://dzone.com/articles/size-entry-map shows 81.4 bytes per concurrentHashmap entry, do you think this is accurate and want me to add that?

Contributor

Jackie-Jiang Apr 29, 2022

In order to make more accurate estimation, we should count the object overhead because as you can see, the object overhead can be larger than the actual content within the entry.
I don't know the formula of object overhead from map size, so we need to do some research here. If you find that is too hard, we may add a TODO and address that later. But users should know this size does not include the overhead, so it can be quite off.

Contributor Author

deemoliu May 2, 2022

Thanks for suggestion! I can add a Todo and address later. I will do some benchmark with heap dump.

pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/Constants.java Outdated Show resolved Hide resolved

pinot-spi/src/main/java/org/apache/pinot/spi/utils/builder/ControllerRequestURLBuilder.java Outdated Show resolved Hide resolved

deemoliu added 5 commits

May 2, 2022 13:55


          add a experiment API for upsert heap memory estimation

cf3b320


          fix test failure, address code comments

f10e1df


          revert inproper merges


          address code comments

489d11b


          address code comment, add TODO

f00f66d

deemoliu force-pushed the qiaochu/capacity-estimate-api branch from 4bfc9aa to f00f66d Compare

May 2, 2022 21:26

Jackie-Jiang approved these changes

View reviewed changes

Jackie-Jiang reviewed

View reviewed changes

Contributor

Jackie-Jiang left a comment

Please update the release note of the PR description for the new added rest endpoint

...ller/src/main/java/org/apache/pinot/controller/api/resources/PinotUpsertRestletResource.java Outdated Show resolved Hide resolved

Jackie-Jiang added the release-notes label


          Update pinot-controller/src/main/java/org/apache/pinot/controller/api…

49f5b90

…/resources/PinotUpsertRestletResource.java

Co-authored-by: Xiaotian (Jackie) Jiang <[email protected]>

Contributor Author

deemoliu commented May 4, 2022

Please update the release note of the PR description for the new added rest endpoint

Thanks @Jackie-Jiang , done.


          address code comments and fix test

795fa15

Jackie-Jiang merged commit 9873ed9 into apache:master

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels