-
Notifications
You must be signed in to change notification settings - Fork 24.7k
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
Adds tests for cardinality and filter aggregations #23826
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I left some minor comments.
} | ||
} else { | ||
for (long i = 0; i < runLens.size(); i++) { | ||
values.add(runLens.get(i)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that should be s/i/(bucket << p)+i/
public boolean equals(long bucket, HyperLogLogPlusPlus other) { | ||
return Objects.equals(p, other.p) && | ||
Objects.equals(algorithm.get(bucket), other.algorithm.get(bucket)) | ||
&& |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
weird indentation?
} | ||
OpenBitSet other = (OpenBitSet) obj; | ||
return Objects.equals(impl, other.impl); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems to me that this is not needed anymore?
for (int i = 0; i < numDocs; i++) { | ||
if (frequently()) { | ||
// make sure we have more than one segment to test the merge | ||
indexWriter.commit(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you can do indexWriter.getReader().close()
, which will create a segment too but not do a fsync
import java.util.Map; | ||
|
||
public class InternalCardinalityTests extends InternalAggregationTestCase<InternalCardinality> { | ||
private static List<HyperLogLogPlusPlus> algos; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this variable never seems to be read?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is needed so we keep a reference to the HyperLogLogPlusPlus implementations we create in createTestInstance()
so we can close them and release the appropriate BigArray
pages after the test is complete (see the cleanup()
method).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
* master: (137 commits) CONSOLEify the "using scripts" documentation Adds tests for cardinality and filter aggregations Add Backoff policy to azure repository Revert "Adds tests for cardinality and filter aggregations (elastic#23826)" Adds tests for cardinality and filter aggregations (elastic#23826) mute testDifferentRolesMaintainPathOnRestart Replace custom sort field with SortedSetSortField and SortedNumericSortField when possible (elastic#23827) Prevent nodes from joining if newer indices exist in the cluster (elastic#23843) Synchronized CollapseTopFieldDocs with lucenes relatives (elastic#23854) CONSOLEify analysis docs Adapted search_shards rest test to work with Perl To examine an exception in rest tests, the exception should be caught, not ignored Fixed bad YAML in rest tests Fix BootstrapForTesting blowup Ban Boolean#getBoolean Fix language in some docs CONSOLEify lang-analyzer docs Stricter parsing of remote node attribute Fix cross-cluster remote node gateway attributes FieldCapabilitiesRequest should implements Replaceable since it accepts index patterns ...
* master: (146 commits) Introduce single-node discovery Await termination after shutting down executors Fix initialization issue in ElasticsearchException Fix bulk queue size in thread pool docs [DOCS] Remove line about eager loading global ordinals GceDiscoverTests - remove intitial_state_timeout SpecificMasterNodesIT shouldn't use autoMinMasterNodes testRestorePersistentSettings doesn't to mess with discovery settings testDifferentRolesMaintainPathOnRestart shouldn't use auto managing of min master nodes CONSOLEify the "using scripts" documentation Adds tests for cardinality and filter aggregations Add Backoff policy to azure repository Revert "Adds tests for cardinality and filter aggregations (elastic#23826)" Adds tests for cardinality and filter aggregations (elastic#23826) mute testDifferentRolesMaintainPathOnRestart Replace custom sort field with SortedSetSortField and SortedNumericSortField when possible (elastic#23827) Prevent nodes from joining if newer indices exist in the cluster (elastic#23843) Synchronized CollapseTopFieldDocs with lucenes relatives (elastic#23854) CONSOLEify analysis docs Adapted search_shards rest test to work with Perl ...
This is a partial backport of elastic#23826
Relates to #22278