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

Create weights lazily in filter and filters aggregation #26983

Merged
merged 1 commit into from
Oct 12, 2017
Merged

Create weights lazily in filter and filters aggregation #26983

merged 1 commit into from
Oct 12, 2017

Conversation

colings86
Copy link
Contributor

@colings86 colings86 commented Oct 12, 2017

Previous to this change the weights for the filter and filters aggregation were created in the Filter(s)AggregatorFactory which meant that they were created regardless of whether the aggregator actually collects any documents. This meant that for filters that are expensive to initialise, requests would not be quick when the query of the request was (or effectively was) a match_none query.

This change maintains a single Weight instance for each filter across parent buckets but passes a weight supplier to the aggregator instances which will create the weight on first call and then return that instance for subsequent calls.

Previous to this change the weights for the filter and filters aggregation were created in the `Filter(s)AggregatorFactory` which meant that they were created regardless of whether the aggregator actually collects any documents. This meant that for filters that are expensive to initialise, requests would not be quick when the query of the request was (or effectively was) a `match_none` query.

This change maintains a single Weight instance for each filter across parent buckets but passes a weight supplier to the aggregator instances which will create the weight on first call and then return that instance for subsequent calls.
Copy link
Contributor

@jimczi jimczi left a comment

Choose a reason for hiding this comment

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

LGTM

Query filter = filterBuilder.toFilter(context.getQueryShardContext());
weight = contextSearcher.createNormalizedWeight(filter, false);
} catch (IOException e) {
throw new AggregationInitializationException("Failed to initialse filter", e);
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: initialise

@colings86 colings86 merged commit e1679bf into elastic:master Oct 12, 2017
@colings86 colings86 deleted the enhance/lazyFilterAggInitialisation branch October 12, 2017 13:58
jasontedor added a commit that referenced this pull request Oct 12, 2017
* master: (35 commits)
  Create weights lazily in filter and filters aggregation (#26983)
  Use a dedicated ThreadGroup in rest sniffer (#26897)
  Fire global checkpoint sync under system context
  Update by Query is modified to accept short `script` parameter. (#26841)
  Cat shards bytes (#26952)
  Add support for parsing inline script (#23824) (#26846)
  Change default value to true for transpositions parameter of fuzzy query (#26901)
  Adding unreleased 5.6.4 version number to Version.java
  Rename TCPTransportTests to TcpTransportTests (#26954)
  Fix NPE for /_cat/indices when no primary shard (#26953)
  [DOCS] Fixed indentation of the definition list.
  Fix formatting in channel close test
  Check for closed connection while opening
  Clarify systemd overrides
  [DOCS] Plugin Installation for Windows (#21671)
  Painless: add tests for cached boxing (#24163)
  Don't detect source's XContentType in DocumentParser.parseDocument() (#26880)
  Fix handling of paths containing parentheses
  Allow only a fixed-size receive predictor (#26165)
  Add Homebrew instructions to getting started
  ...
colings86 added a commit that referenced this pull request Oct 13, 2017
Previous to this change the weights for the filter and filters aggregation were created in the `Filter(s)AggregatorFactory` which meant that they were created regardless of whether the aggregator actually collects any documents. This meant that for filters that are expensive to initialise, requests would not be quick when the query of the request was (or effectively was) a `match_none` query.

This change maintains a single Weight instance for each filter across parent buckets but passes a weight supplier to the aggregator instances which will create the weight on first call and then return that instance for subsequent calls.
colings86 added a commit that referenced this pull request Oct 13, 2017
Previous to this change the weights for the filter and filters aggregation were created in the `Filter(s)AggregatorFactory` which meant that they were created regardless of whether the aggregator actually collects any documents. This meant that for filters that are expensive to initialise, requests would not be quick when the query of the request was (or effectively was) a `match_none` query.

This change maintains a single Weight instance for each filter across parent buckets but passes a weight supplier to the aggregator instances which will create the weight on first call and then return that instance for subsequent calls.
colings86 added a commit that referenced this pull request Oct 13, 2017
Previous to this change the weights for the filter and filters aggregation were created in the `Filter(s)AggregatorFactory` which meant that they were created regardless of whether the aggregator actually collects any documents. This meant that for filters that are expensive to initialise, requests would not be quick when the query of the request was (or effectively was) a `match_none` query.

This change maintains a single Weight instance for each filter across parent buckets but passes a weight supplier to the aggregator instances which will create the weight on first call and then return that instance for subsequent calls.
jasontedor added a commit to olcbean/elasticsearch that referenced this pull request Oct 16, 2017
* master: (356 commits)
  Do not set SO_LINGER on server channels (elastic#26997)
  Fix inconsistencies in the rest api specs for *_script (elastic#26971)
  fix inconsistencies in the rest api specs for cat.snapshots (elastic#26996)
  Add docs on full_id parameter in cat nodes API
  [TEST] Add test that replicates versioned updates with random flushes
  Use internal searcher for all indexing related operations in the engine
  Reformat paragraph in template docs to 80 columns
  Clarify settings and template on create index
  Fix reference to TcpTransport in documentation
  Allow Uid#decodeId to decode from a byte array slice (elastic#26987)
  Fix a typo in the similarity docs (elastic#26970)
  Use separate searchers for "search visibility" vs "move indexing buffer to disk (elastic#26972)
  Create weights lazily in filter and filters aggregation (elastic#26983)
  Use a dedicated ThreadGroup in rest sniffer (elastic#26897)
  Fire global checkpoint sync under system context
  Update by Query is modified to accept short `script` parameter. (elastic#26841)
  Cat shards bytes (elastic#26952)
  Add support for parsing inline script (elastic#23824) (elastic#26846)
  Change default value to true for transpositions parameter of fuzzy query (elastic#26901)
  Adding unreleased 5.6.4 version number to Version.java
  ...
jasontedor added a commit that referenced this pull request Oct 16, 2017
* 6.x:
  Remove unnecessary exception for engine constructor
  Update docs about `script` parameter (#27010)
  Do not set SO_LINGER on server channels (#26997)
  Fix inconsistencies in the rest api specs for *_script (#26971)
  fix inconsistencies in the rest api specs for cat.snapshots (#26996)
  Add docs on full_id parameter in cat nodes API
  Add removal of types to the 6.0 breaking changes
  Create weights lazily in filter and filters aggregation (#26983)
  [TEST] Add test that replicates versioned updates with random flushes
  Use internal searcher for all indexing related operations in the engine
  Use separate searchers for "search visibility" vs "move indexing buffer to disk (#26972)
  Reformat paragraph in template docs to 80 columns
  Clarify settings and template on create index
  Fix reference to TcpTransport in documentation
  Allow Uid#decodeId to decode from a byte array slice (#26987)
  Fix a typo in the similarity docs (#26970)
@lcawl lcawl added v6.0.0-rc2 and removed v6.0.0 labels Oct 30, 2017
@lcawl lcawl removed the v6.1.0 label Dec 12, 2017
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