Skip to content

Conversation

@klsince
Copy link
Contributor

@klsince klsince commented Aug 13, 2021

Description

This method expose the column indices created in local segment folder, which can be compared with indices in table config to decide which indices to remove as to be implemented in #7286.

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

@codecov-commenter
Copy link

codecov-commenter commented Aug 13, 2021

Codecov Report

Merging #7297 (4596b74) into master (fca88a2) will decrease coverage by 56.67%.
The diff coverage is 0.00%.

❗ Current head 4596b74 differs from pull request most recent head 0377e46. Consider uploading reports for the commit 0377e46 to get more accurate results
Impacted file tree graph

@@              Coverage Diff              @@
##             master    #7297       +/-   ##
=============================================
- Coverage     71.31%   14.63%   -56.68%     
+ Complexity     3361       92     -3269     
=============================================
  Files          1503     1457       -46     
  Lines         73995    72100     -1895     
  Branches      10709    10510      -199     
=============================================
- Hits          52766    10550    -42216     
- Misses        17627    60784    +43157     
+ Partials       3602      766     -2836     
Flag Coverage Δ
integration1 ?
integration2 ?
unittests1 ?
unittests2 14.63% <0.00%> (-0.02%) ⬇️

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

Impacted Files Coverage Δ
...ent/local/segment/store/FilePerIndexDirectory.java 0.00% <0.00%> (-91.77%) ⬇️
...t/local/segment/store/SegmentLocalFSDirectory.java 0.00% <0.00%> (-68.85%) ⬇️
.../local/segment/store/SingleFileIndexDirectory.java 0.00% <0.00%> (-87.12%) ⬇️
.../pinot/segment/spi/store/ColumnIndexDirectory.java 0.00% <ø> (-66.67%) ⬇️
...ache/pinot/segment/spi/store/SegmentDirectory.java 0.00% <ø> (-83.34%) ⬇️
...c/main/java/org/apache/pinot/common/tier/Tier.java 0.00% <0.00%> (-100.00%) ⬇️
...ain/java/org/apache/pinot/core/data/table/Key.java 0.00% <0.00%> (-100.00%) ⬇️
.../java/org/apache/pinot/core/data/table/Record.java 0.00% <0.00%> (-100.00%) ⬇️
.../java/org/apache/pinot/core/util/GroupByUtils.java 0.00% <0.00%> (-100.00%) ⬇️
...ava/org/apache/pinot/spi/data/MetricFieldSpec.java 0.00% <0.00%> (-100.00%) ⬇️
... and 1184 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 fca88a2...0377e46. Read the comment docs.

@klsince klsince force-pushed the get_column_indices branch from 357022a to 017b9be Compare August 13, 2021 16:16
Copy link
Contributor

@Jackie-Jiang Jackie-Jiang left a comment

Choose a reason for hiding this comment

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

LGTM

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.

3 participants