Skip to content

Conversation

@klsince
Copy link
Contributor

@klsince klsince commented Feb 18, 2022

Allow to set segment name postfix when using SegmentProcessorFramework, right now that's hard coded as null.
Meanwhile, fix a bug in NormalizedDate name generator, right now it doesn't join postfix into segment name.

Description

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

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

@codecov-commenter
Copy link

codecov-commenter commented Feb 18, 2022

Codecov Report

Merging #8230 (aa65c93) into master (287f552) will decrease coverage by 6.67%.
The diff coverage is 100.00%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master    #8230      +/-   ##
============================================
- Coverage     71.01%   64.33%   -6.68%     
  Complexity     4320     4320              
============================================
  Files          1626     1581      -45     
  Lines         85067    83194    -1873     
  Branches      12799    12598     -201     
============================================
- Hits          60408    53523    -6885     
- Misses        20505    25825    +5320     
+ Partials       4154     3846     -308     
Flag Coverage Δ
integration1 ?
integration2 ?
unittests1 67.34% <100.00%> (-0.01%) ⬇️
unittests2 14.14% <5.26%> (+<0.01%) ⬆️

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

Impacted Files Coverage Δ
.../org/apache/pinot/core/common/MinionConstants.java 0.00% <ø> (ø)
...re/segment/processing/framework/SegmentConfig.java 90.00% <100.00%> (+2.50%) ⬆️
...rocessing/framework/SegmentProcessorFramework.java 98.52% <100.00%> (+0.04%) ⬆️
...ache/pinot/plugin/minion/tasks/MergeTaskUtils.java 96.07% <100.00%> (+0.07%) ⬆️
...eator/name/NormalizedDateSegmentNameGenerator.java 92.45% <100.00%> (+4.21%) ⬆️
...a/org/apache/pinot/common/metrics/MinionMeter.java 0.00% <0.00%> (-100.00%) ⬇️
...g/apache/pinot/common/metrics/ControllerMeter.java 0.00% <0.00%> (-100.00%) ⬇️
.../apache/pinot/common/metrics/BrokerQueryPhase.java 0.00% <0.00%> (-100.00%) ⬇️
.../apache/pinot/common/metrics/MinionQueryPhase.java 0.00% <0.00%> (-100.00%) ⬇️
...ache/pinot/server/access/AccessControlFactory.java 0.00% <0.00%> (-100.00%) ⬇️
... and 374 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 287f552...aa65c93. Read the comment docs.

@Jackie-Jiang Jackie-Jiang merged commit a9b64da into apache:master Feb 19, 2022
xiangfu0 pushed a commit to xiangfu0/pinot that referenced this pull request Feb 23, 2022
Allow to set segment name postfix when using SegmentProcessorFramework, right now that's hard coded as null.
Meanwhile, fix a bug in NormalizedDate name generator, right now it doesn't join postfix into segment name.
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