Skip to content

Add test for ReservoirSegmentSampler#14591

Merged
kfaraz merged 3 commits intoapache:masterfrom
kfaraz:add_reservoir_sampling_test
Jul 17, 2023
Merged

Add test for ReservoirSegmentSampler#14591
kfaraz merged 3 commits intoapache:masterfrom
kfaraz:add_reservoir_sampling_test

Conversation

@kfaraz
Copy link
Contributor

@kfaraz kfaraz commented Jul 16, 2023

Tests to verify the following behaviour have been added:

  • Segments from more populous servers are more likely to be picked irrespective of sample size.
  • Segments from all servers are equally likely to be picked if all servers have equivalent number of segments

Copy link
Contributor

@AmatyaAvadhanula AmatyaAvadhanula left a comment

Choose a reason for hiding this comment

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

Thank you, @kfaraz. LGTM.

@kfaraz
Copy link
Contributor Author

kfaraz commented Jul 17, 2023

Thanks for the review, @AmatyaAvadhanula ! I have added some more comments as discussed with you offline.

@kfaraz kfaraz merged commit ab051d9 into apache:master Jul 17, 2023
@kfaraz kfaraz deleted the add_reservoir_sampling_test branch July 17, 2023 13:20
sergioferragut pushed a commit to sergioferragut/druid that referenced this pull request Jul 21, 2023
Tests to verify the following behaviour have been added:
- Segments from more populous servers are more likely to be picked irrespective of
sample size.
- Segments from all servers are equally likely to be picked if all servers have equivalent
number of segments.
@LakshSingla LakshSingla added this to the 28.0 milestone Oct 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants