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

Use a 1024 byte minimum weight for filter cache entries #8304

Merged
merged 1 commit into from Oct 31, 2014

Conversation

dakrone
Copy link
Member

@dakrone dakrone commented Oct 31, 2014

This changes the weighing function for the filter cache to use a
configurable minimum weight for each filter cached. This value defaults
to 1kb and can be configured with the
indices.cache.filter.minimum_entry_weight setting.

This also fixes an issue with the filter cache where the concurrency
level of the cache was exposed as a setting, but not used in cache
construction.

Relates to #8268
Fixes #8249

@dakrone
Copy link
Member Author

dakrone commented Oct 31, 2014

I also manually tested this, where I ran a shell script containing non-existing term filters over the wikipedia corpus, I was able to see:

        "filter_cache" : {
          "memory_size" : "396.2kb",
          "memory_size_in_bytes" : 405760,
          "evictions" : 55018
        }

to show that filter cache entries are being evicted even though the size limit has not been reached due to the minimum size.

@@ -145,6 +154,7 @@ public void addReaderKeyToClean(Object readerKey) {
public void close() {
closed = true;
cache.invalidateAll();
cache.cleanUp();
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it needed because of this change or is it unrelated?

Copy link
Member Author

Choose a reason for hiding this comment

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

Not needed, but always good to actually clean the cache when closing, happy to remove it if you'd like

Copy link
Contributor

Choose a reason for hiding this comment

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

I was just wondering if there could be bad side-effects such as causing some operations to take longer.

Copy link
Member Author

Choose a reason for hiding this comment

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

There aren't any bad side-effects, without this, if the cache were closed and everything was invalidated, they would not be immediately removed, this just immediately removes them.

@jpountz
Copy link
Contributor

jpountz commented Oct 31, 2014

LGTM

This changes the weighing function for the filter cache to use a
configurable minimum weight for each filter cached. This value defaults
to 1kb and can be configured with the
`indices.cache.filter.minimum_entry_weight` setting.

This also fixes an issue with the filter cache where the concurrency
level of the cache was exposed as a setting, but not used in cache
construction.

Relates to elastic#8268
@dakrone dakrone merged commit 42b6e01 into elastic:master Oct 31, 2014
@clintongormley clintongormley changed the title Use a 1024 byte minimum weight for filter cache entries Internal: Use a 1024 byte minimum weight for filter cache entries Nov 3, 2014
@dakrone dakrone deleted the bound-filter-cache-size branch November 11, 2014 12:47
@clintongormley clintongormley changed the title Internal: Use a 1024 byte minimum weight for filter cache entries Use a 1024 byte minimum weight for filter cache entries 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.

1.3.4 heap used growing and crashing ES
3 participants