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

Allow the query cache to be disabled. #16268

Merged
merged 1 commit into from Apr 11, 2016

Conversation

jpountz
Copy link
Contributor

@jpountz jpountz commented Jan 27, 2016

This feature already existed but was neither tested nor documented.

Closes #15802

@nik9000
Copy link
Member

nik9000 commented Jan 27, 2016

LGTM

@rjernst
Copy link
Member

rjernst commented Jan 27, 2016

Is having a custom query cache really something we should be promoting? It seems like allowing to disable it is one thing, and should be done with a boolean.

@jpountz
Copy link
Contributor Author

jpountz commented Jan 27, 2016

I enabled it the way it was already implemented but I agree we probably want to keep things more contained.

@jpountz
Copy link
Contributor Author

jpountz commented Apr 7, 2016

Thanks @rjernst for the suggestion. I replaced the setting with a flag and removed the ability to configure custom query caches.

@rjernst
Copy link
Member

rjernst commented Apr 11, 2016

This LGTM as is. I have 2 thoughts for the future:

  • Now that "none" is gone as far as the user is concerned, I wonder if we could rename the impl to a name that makes sense. Perhaps DisabledQueryCache?
  • It would be nice if we removed the force method and used a package private member that tests could override, like we do with other test only overrides.

@jpountz
Copy link
Contributor Author

jpountz commented Apr 11, 2016

DisabledQueryCache

I like the name better too so I will rename before merging.

Thanks!

This replaces the internal `index.queries.cache.type` setting with
a new `index.queries.cache.enabled` setting, which is documented.

Closes elastic#15802
@jpountz jpountz force-pushed the feature/disable_query_cache branch from 83f2e08 to 0eb1a81 Compare April 11, 2016 16:06
@jpountz jpountz merged commit 0eb1a81 into elastic:master Apr 11, 2016
@jpountz jpountz deleted the feature/disable_query_cache branch April 11, 2016 16:08
russcam added a commit to elastic/elasticsearch-net that referenced this pull request Jun 14, 2016
russcam added a commit to elastic/elasticsearch-net that referenced this pull request Jun 16, 2016
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

4 participants