Skip to content

Remove/Deprecate HLC handling code#11590

Merged
Jackie-Jiang merged 2 commits intoapache:masterfrom
Jackie-Jiang:remove_hlc_handling
Sep 16, 2023
Merged

Remove/Deprecate HLC handling code#11590
Jackie-Jiang merged 2 commits intoapache:masterfrom
Jackie-Jiang:remove_hlc_handling

Conversation

@Jackie-Jiang
Copy link
Contributor

  • Removed classes:
    • HLCSegmentName
    • SegmentName.RealtimeSegmentType
    • PinotRealtimeSegmentManager
    • ReplicationUtils
    • StreamConfig.ConsumerType
  • Deprecated classes:
    • SegmentName
    • PartitionLevelStreamConfig

@Jackie-Jiang Jackie-Jiang added incompatible Indicate PR that introduces backward incompatibility cleanup labels Sep 14, 2023
@codecov-commenter
Copy link

codecov-commenter commented Sep 14, 2023

Codecov Report

Merging #11590 (246f93e) into master (a8b5fe7) will increase coverage by 0.10%.
Report is 1 commits behind head on master.
The diff coverage is 69.64%.

@@             Coverage Diff              @@
##             master   #11590      +/-   ##
============================================
+ Coverage     63.06%   63.16%   +0.10%     
+ Complexity     1107     1106       -1     
============================================
  Files          2326     2323       -3     
  Lines        124928   124405     -523     
  Branches      19073    18981      -92     
============================================
- Hits          78787    78586     -201     
+ Misses        40542    40239     -303     
+ Partials       5599     5580      -19     
Flag Coverage Δ
integration <0.01% <0.00%> (+<0.01%) ⬆️
integration1 <0.01% <0.00%> (+<0.01%) ⬆️
integration2 0.00% <0.00%> (ø)
java-11 63.14% <69.64%> (+0.10%) ⬆️
java-17 63.03% <69.64%> (+0.13%) ⬆️
java-20 63.03% <69.64%> (+0.12%) ⬆️
temurin 63.16% <69.64%> (+0.10%) ⬆️
unittests 63.16% <69.64%> (+0.10%) ⬆️
unittests1 67.44% <70.58%> (-0.04%) ⬇️
unittests2 14.51% <33.92%> (+0.01%) ⬆️

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

Files Changed Coverage Δ
...ache/pinot/common/metadata/ZKMetadataProvider.java 11.50% <0.00%> (-0.12%) ⬇️
...ava/org/apache/pinot/common/utils/SegmentName.java 0.00% <0.00%> (-85.19%) ⬇️
...ler/api/resources/PinotSegmentRestletResource.java 16.02% <0.00%> (ø)
.../controller/helix/core/SegmentDeletionManager.java 73.29% <ø> (+0.89%) ⬆️
.../helix/core/realtime/SegmentCompletionManager.java 70.67% <0.00%> (ø)
...realtime/segment/DefaultFlushThresholdUpdater.java 100.00% <ø> (ø)
...ealtime/segment/SegmentFlushThresholdComputer.java 95.71% <ø> (ø)
...ntroller/helix/core/rebalance/TableRebalancer.java 68.85% <ø> (+0.93%) ⬆️
...roller/helix/core/relocation/SegmentRelocator.java 51.14% <0.00%> (+1.14%) ⬆️
...gments/RealtimeToOfflineSegmentsTaskGenerator.java 95.41% <ø> (+2.03%) ⬆️
... and 22 more

... and 11 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@xiangfu0
Copy link
Contributor

I think we also want to deprecate _replicasPerPartition in SegmentsValidationAndRetentionConfig?

@Jackie-Jiang Jackie-Jiang merged commit e26a29e into apache:master Sep 16, 2023
@Jackie-Jiang Jackie-Jiang deleted the remove_hlc_handling branch September 16, 2023 01:50
Copy link
Contributor

@walterddr walterddr left a comment

Choose a reason for hiding this comment

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

missed this PR but this is AWESOME!!!!

int numSeparators = getNumSeparators(segmentName);
return numSeparators == 2 || numSeparators == 4;
@Deprecated
public class SegmentName {
Copy link
Contributor

Choose a reason for hiding this comment

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

this class can be removed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Kept SegmentName and PartitionLevelStreamConfig for compatibility. Will remove in next release

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

Labels

cleanup incompatible Indicate PR that introduces backward incompatibility

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants