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

Better heuristic for setting default shard_size in terms aggregation #6960

Closed
wants to merge 0 commits into from
Closed

Better heuristic for setting default shard_size in terms aggregation #6960

wants to merge 0 commits into from

Conversation

colings86
Copy link
Contributor

The default shard size in the terms aggregation now uses BucketUtils.suggestShardSideQueueSize() to set the shard size if the user does not specify it as a parameter.

Closes #6857

@@ -51,6 +52,11 @@ public AggregatorFactory parse(String aggregationName, XContentParser parser, Se
aggParser.parse(aggregationName, parser, context, vsParser, incExcParser);

TermsAggregator.BucketCountThresholds bucketCountThresholds = aggParser.getBucketCountThresholds();
if (bucketCountThresholds.getShardSize() == aggParser.getDefaultBucketCountThresholds().getShardSize()) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we should not do it when the order is based on the term?

@jpountz
Copy link
Contributor

jpountz commented Jul 23, 2014

Left one comment but it looks good!

@jpountz
Copy link
Contributor

jpountz commented Jul 24, 2014

LGTM

@colings86
Copy link
Contributor Author

Merged into master and 1.x

@colings86 colings86 closed this Jul 24, 2014
@jpountz jpountz removed the review label Jul 24, 2014
@colings86 colings86 self-assigned this Aug 21, 2014
@colings86 colings86 deleted the fix/6857 branch August 21, 2014 15:11
@clintongormley clintongormley changed the title Aggregations: change to default shard_size in terms aggregation Aggregations: Change to default shard_size in terms aggregation Sep 10, 2014
@javanna
Copy link
Member

javanna commented Feb 18, 2015

Hey @colings86 can we update our docs according to this change?

@colings86
Copy link
Contributor Author

Docs updated in #9873

@clintongormley clintongormley changed the title Aggregations: Change to default shard_size in terms aggregation Better heuristic for setting default shard_size in terms aggregation Jun 7, 2015
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.

Aggregations: Better shard_size default for terms aggregation
4 participants