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

Store filter cache statistics at the shard level instead of index. #11886

Merged
merged 1 commit into from Jun 29, 2015

Conversation

jpountz
Copy link
Contributor

@jpountz jpountz commented Jun 26, 2015

Today we keep track of how often filters are used at the index level in order
to decide whether they should be cached or not. This is an issue if you have
several shards of the same index on the same node as it will multiply statistics
by the number of shards that you have for this index on the node, which defeats
the purpose of waiting for a filter to be reused before caching it.

@bleskes
Copy link
Contributor

bleskes commented Jun 29, 2015

I'm good with the change, I just wonder if we should use Guice to instantiate it. We don't expose custom classes setting anyway. Can we instantiate it in the IndexShard constructor? This means that we could pass in some settings in the future (like the minimum segment size etc.)

@jpountz jpountz force-pushed the fix/shard_filter_cache_stats branch from 14da97c to bbc27ad Compare June 29, 2015 10:58
@jpountz
Copy link
Contributor Author

jpountz commented Jun 29, 2015

Thanks @bleskes you made a very good point. I rebased against master which changed a bit after the cache renamings, and applied your suggestion.

@@ -19,6 +19,8 @@

package org.elasticsearch.index.shard;

import org.apache.lucene.search.QueryCachingPolicy;
Copy link
Contributor

Choose a reason for hiding this comment

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

unneeded?

Today we keep track of how often filters are used at the index level in order
to decide whether they should be cached or not. This is an issue if you have
several shards of the same index on the same node as it will multiply statistics
by the number of shards that you have for this index on the node, which defeats
the purpose of waiting for a filter to be reused before caching them.
@jpountz jpountz force-pushed the fix/shard_filter_cache_stats branch from 8b8948d to ccaf846 Compare June 29, 2015 11:34
jpountz added a commit that referenced this pull request Jun 29, 2015
Store filter cache statistics at the shard level instead of index.
@jpountz jpountz merged commit 18b73ce into elastic:master Jun 29, 2015
@kevinkluge kevinkluge removed the review label Jun 29, 2015
@jpountz jpountz deleted the fix/shard_filter_cache_stats branch June 29, 2015 11:35
@bleskes
Copy link
Contributor

bleskes commented Jun 29, 2015

for completeness - LGTM :)

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.

None yet

3 participants