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

Fix random sampler consistency test #107957

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

benwtrent
Copy link
Member

Random sampler consistency requires a restricted number of segments, to ensure we always hit the same number of segments and that no merging is occurring, this merges the segment count to 1 for this particular test.

In practice, this isn't needed as the approximate nature of the aggregation already means you could get different statistics per call, but they are within an error bound set by the users configured sampling probability.

closes: #105839

@benwtrent benwtrent added >test Issues or PRs that are addressing/adding tests :Search/Search Search-related issues that do not fall into other categories v8.15.0 labels Apr 26, 2024
@elasticsearchmachine elasticsearchmachine added the Team:Search Meta label for search team label Apr 26, 2024
@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/es-search (Team:Search)

@benwtrent
Copy link
Member Author

@elasticmachine update branch

public void testRandomSamplerConsistentSeed() {
double[] sampleMonotonicValue = new double[1];
double[] sampleNumericValue = new double[1];
long[] sampledDocCount = new long[1];
// Force merge to ensure segment consistency as any segment merging can change which particular documents
// are sampled
assertNoFailures(indicesAdmin().prepareForceMerge("idx").setMaxNumSegments(1).get());
Copy link
Contributor

Choose a reason for hiding this comment

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

I see that idx index is setup and indexed once for all tests in this module. Is this correct? If so, I have 2 questions:

  • Is it being force-merged to 1 segment having any influence on other tests?
  • Does it make sense to move force-merge to setupSuiteScopeCluster()?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Search/Search Search-related issues that do not fall into other categories Team:Search Meta label for search team >test Issues or PRs that are addressing/adding tests v8.15.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[CI] RandomSamplerIT testRandomSamplerConsistentSeed failing
4 participants