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.

Sign up for GitHub

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

Jump to bottom

Core: Rename caches #11569

Closed
jpountz opened this issue Jun 10, 2015 · 9 comments
Closed

Core: Rename caches #11569

jpountz opened this issue Jun 10, 2015 · 9 comments
Assignees
@jpountz
Labels
blocker v2.0.0-beta1

Comments

@jpountz
Copy link
Contributor

jpountz commented Jun 10, 2015

With the merge of queries and filters, it would make sense to rename our caches:

  • filter cache -> query cache
  • shard query cache -> shard request cache

I also have hopes that the latter renaming will help remove confusion that the request cache is short-lived since it is invalidated on every refresh.
@jpountz jpountz self-assigned this Jun 10, 2015
@jpountz jpountz changed the title Core: Rename cache names Core: Rename cache Jun 10, 2015
@jpountz jpountz changed the title Core: Rename cache Core: Rename caches Jun 10, 2015
@s1monw
Copy link
Contributor

s1monw commented Jun 10, 2015

++

@jpountz
Copy link
Contributor Author

jpountz commented Jun 10, 2015

One challenge here is that the new setting for the size of the query cache would be index.cache.query.size while it is also the old setting for the request cache size. So we need somehow to be able to differenciate the two.

@s1monw
Copy link
Contributor

s1monw commented Jun 11, 2015

One challenge here is that the new setting for the size of the query cache would be index.cache.query.size while it is also the old setting for the request cache size. So we need somehow to be able to differentiate the two.

yeah we can't really... we can go to index.shard.query.cache.size maybe?

@clintongormley
Copy link

And node.cache.query.size given that it is node level?

@bleskes
Copy link
Contributor

bleskes commented Jun 12, 2015

we typically prefix the node level index related settings with indices. I like the cache prefix as well.

How about indices.cache.shard_requests.* (using adrien’s naming of shard request cache)?

On 12 Jun 2015, at 16:13, Clinton Gormley notifications@github.com wrote:

And node.cache.query.size given that it is node level?

—
Reply to this email directly or view it on GitHub.

@jpountz
Copy link
Contributor Author

jpountz commented Jun 15, 2015

@bleskes That works for me, but the issue here is not the new name of the request cache, but the new name of the query cache (formerly known as filter cache) so that it would be different from the previous name of the request cache.

@bleskes
Copy link
Contributor

bleskes commented Jun 15, 2015

@jpountz yeah, I got confused. The important part for me was to point out the usage of "indices." as node level settings which related to indices.

As far as the name collision goes - I wonder how many people use the current query cache (i.e. shard requests caching) and thus how bad it would be to make a breaking change and change the semantics of that settings.

Another alternative, which I'm on the fence about but think it's worth mentioning, is using the word segments in the name - as the queries are cached on a segment level. So that will be indices.cache.segment.query & indices.cache.shard.requests .

@jpountz
Copy link
Contributor Author

jpountz commented Jun 18, 2015

If there are no objections, I will try to implement Boaz's last proposal.

@s1monw
Copy link
Contributor

s1monw commented Jun 22, 2015

If there are no objections, I will try to implement Boaz's last proposal.

++

@jpountz jpountz mentioned this issue Jun 26, 2015
Merged
jpountz added a commit to jpountz/elasticsearch that referenced this issue Jun 29, 2015
@jpountz
Rename caches.
38f5cc2 
In order to be more consistent with what they do, the query cache has been
renamed to request cache and the filter cache has been renamed to query
cache.

A known issue is that package/logger names do no longer match settings names,
please speak up if you think this is an issue.

Here are the settings for which I kept backward compatibility. Note that they
are a bit different from what was discussed on elastic#11569 but putting `cache` before
the name of what is cached has the benefit of making these settings consistent
with the fielddata cache whose size is configured by
`indices.fielddata.cache.size`:
 * index.cache.query.enable -> index.requests.cache.enable
 * indices.cache.query.size -> indices.requests.cache.size
 * indices.cache.filter.size -> indices.queries.cache.size

Close elastic#11569
szroland pushed a commit to szroland/elasticsearch that referenced this issue Jun 30, 2015
@jpountz @szroland
Rename caches.
7b0c83b 
In order to be more consistent with what they do, the query cache has been
renamed to request cache and the filter cache has been renamed to query
cache.

A known issue is that package/logger names do no longer match settings names,
please speak up if you think this is an issue.

Here are the settings for which I kept backward compatibility. Note that they
are a bit different from what was discussed on elastic#11569 but putting `cache` before
the name of what is cached has the benefit of making these settings consistent
with the fielddata cache whose size is configured by
`indices.fielddata.cache.size`:
 * index.cache.query.enable -> index.requests.cache.enable
 * indices.cache.query.size -> indices.requests.cache.size
 * indices.cache.filter.size -> indices.queries.cache.size

Close elastic#11569
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@jpountz jpountz

Labels
blocker v2.0.0-beta1
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@clintongormley @jpountz @s1monw @bleskes