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

Improve the proactive segment clean-up for REVERTED #8071

Merged
merged 1 commit into from
Jan 27, 2022

Conversation

snleee
Copy link
Contributor

@snleee snleee commented Jan 26, 2022

Current code did not handle the proactive clean-up correctly
for lineage entries with REVERTED state.

  • Added the proactive clean-up for 'segmentsTo' for REVERTED
    during the startSegmentReplacement API call.
  • Added the proactive clean-up for 'segmentsTo' during the
    revertSegmentReplacement API call.
  • Added unit test.

@snleee snleee force-pushed the covering-segment-cleanup-reverted branch from 558828c to 17ef9bc Compare January 26, 2022 03:30
@snleee snleee force-pushed the covering-segment-cleanup-reverted branch 2 times, most recently from f0e9270 to aec5e90 Compare January 26, 2022 05:30
@codecov-commenter
Copy link

Codecov Report

Merging #8071 (70f39af) into master (cc2f3fe) will increase coverage by 3.24%.
The diff coverage is n/a.

❗ Current head 70f39af differs from pull request most recent head aec5e90. Consider uploading reports for the commit aec5e90 to get more accurate results
Impacted file tree graph

@@             Coverage Diff              @@
##             master    #8071      +/-   ##
============================================
+ Coverage     64.63%   67.88%   +3.24%     
+ Complexity     4260     4180      -80     
============================================
  Files          1562     1211     -351     
  Lines         81525    60564   -20961     
  Branches      12252     9347    -2905     
============================================
- Hits          52695    41113   -11582     
+ Misses        25072    16552    -8520     
+ Partials       3758     2899     -859     
Flag Coverage Δ
unittests1 67.88% <ø> (+0.01%) ⬆️
unittests2 ?

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

Impacted Files Coverage Δ
.../aggregation/function/ModeAggregationFunction.java 88.10% <0.00%> (-0.28%) ⬇️
...ntroller/helix/core/PinotHelixResourceManager.java
.../assignment/instance/InstanceAssignmentDriver.java
...he/pinot/ingestion/utils/JobPreparationHelper.java
.../org/apache/pinot/client/PinotClientException.java
...ache/pinot/broker/queryquota/QueryQuotaEntity.java
...ot/controller/tuner/NoOpTableTableConfigTuner.java
...er/recommender/io/metadata/SchemaWithMetaData.java
...ot/controller/util/ConsumingSegmentInfoReader.java
...pinot/minion/event/DefaultMinionEventObserver.java
... and 345 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 cc2f3fe...aec5e90. Read the comment docs.

@jtao15
Copy link
Contributor

jtao15 commented Jan 26, 2022

LGTM, thanks for the improvement!

Copy link
Member

@jackjlli jackjlli left a comment

Choose a reason for hiding this comment

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

Minor but LGTM. Thanks for making the changes!

Current code did not handle the proactive clean-up correctly
for lineage entries with REVERTED state.

- Added the proactive clean-up for 'segmentsTo' for REVERTED
  during the startSegmentReplacement API call.
- Added the proactive clean-up for 'segmentsTo' during the
  revertSegmentReplacement API call.
- Added unit test.
@snleee snleee force-pushed the covering-segment-cleanup-reverted branch from aec5e90 to 45de990 Compare January 27, 2022 05:21
@jackjlli jackjlli merged commit 4d10e03 into apache:master Jan 27, 2022
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