Skip to content
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.

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

Add getSelectedSegments API #7651

Merged
merged 1 commit into from
Oct 28, 2021
Merged

Add getSelectedSegments API #7651

merged 1 commit into from
Oct 28, 2021

Conversation

jackjlli
Copy link
Member

@jackjlli jackjlli commented Oct 27, 2021

Description

This PR adds getSelectedSegments API in PinotSegmentRestletResource class. This API is useful to get the segments which are used for serving queries (i.e. the set of segments that are used in routing table) given the start and end timestamps.
If the table is a refresh use case, the value of start and end timestamp is voided, since there is no time column for refresh use case; instead, the whole qualified segments will be returned.

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

@jackjlli jackjlli force-pushed the add-select-segment-api branch 2 times, most recently from bd0d4d1 to 4b0fa22 Compare October 28, 2021 00:15
Copy link
Contributor

@snleee snleee left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add some response examples to the PR message? Also, please double-check on the test.

Otherwise, LGTM

@jackjlli jackjlli force-pushed the add-select-segment-api branch from 4b0fa22 to 52171bf Compare October 28, 2021 16:57
@jackjlli jackjlli force-pushed the add-select-segment-api branch from 52171bf to 04de569 Compare October 28, 2021 17:09
@codecov-commenter
Copy link

codecov-commenter commented Oct 28, 2021

Codecov Report

Merging #7651 (8bb7697) into master (0166135) will decrease coverage by 40.80%.
The diff coverage is 0.00%.

❗ Current head 8bb7697 differs from pull request most recent head 04de569. Consider uploading reports for the commit 04de569 to get more accurate results
Impacted file tree graph

@@              Coverage Diff              @@
##             master    #7651       +/-   ##
=============================================
- Coverage     71.75%   30.94%   -40.81%     
=============================================
  Files          1576     1567        -9     
  Lines         79966    79684      -282     
  Branches      11843    11826       -17     
=============================================
- Hits          57377    24661    -32716     
- Misses        18730    52921    +34191     
+ Partials       3859     2102     -1757     
Flag Coverage Δ
integration1 29.35% <0.00%> (-0.04%) ⬇️
integration2 27.80% <0.00%> (+0.13%) ⬆️
unittests1 ?
unittests2 ?

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

Impacted Files Coverage Δ
...ler/api/resources/PinotSegmentRestletResource.java 20.00% <0.00%> (-3.96%) ⬇️
...ntroller/helix/core/PinotHelixResourceManager.java 45.73% <0.00%> (-19.66%) ⬇️
...loader/defaultcolumn/BaseDefaultColumnHandler.java 0.00% <0.00%> (-67.50%) ⬇️
...c/main/java/org/apache/pinot/common/tier/Tier.java 0.00% <0.00%> (-100.00%) ⬇️
.../java/org/apache/pinot/spi/utils/BooleanUtils.java 0.00% <0.00%> (-100.00%) ⬇️
...ava/org/apache/pinot/spi/data/MetricFieldSpec.java 0.00% <0.00%> (-100.00%) ⬇️
...va/org/apache/pinot/spi/utils/BigDecimalUtils.java 0.00% <0.00%> (-100.00%) ⬇️
...java/org/apache/pinot/common/tier/TierFactory.java 0.00% <0.00%> (-100.00%) ⬇️
...a/org/apache/pinot/spi/config/table/TableType.java 0.00% <0.00%> (-100.00%) ⬇️
.../org/apache/pinot/spi/data/DimensionFieldSpec.java 0.00% <0.00%> (-100.00%) ⬇️
... and 1062 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 0166135...04de569. Read the comment docs.

@jackjlli jackjlli merged commit 127f4c3 into master Oct 28, 2021
@jackjlli jackjlli deleted the add-select-segment-api branch October 28, 2021 18:17
@snleee
Copy link
Contributor

snleee commented Nov 22, 2021

#7813

kriti-sc pushed a commit to kriti-sc/incubator-pinot that referenced this pull request Dec 12, 2021
Co-authored-by: Jack Li(Analytics Engineering) <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants