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 a plugin to supply its own query cache implementation #12881

Merged
merged 1 commit into from Aug 17, 2015

Conversation

Projects
None yet
5 participants
@martijnvg
Member

martijnvg commented Aug 14, 2015

No description provided.

@rjernst

View changes

Show outdated Hide outdated core/src/main/java/org/elasticsearch/index/cache/IndexCacheModule.java Outdated
@martijnvg

This comment has been minimized.

Show comment
Hide comment
@martijnvg

martijnvg Aug 16, 2015

Member

@uboness @rjernst I've updated the PR use ExtensionPoint to register query cache implementations.

I think the fact that a plugin can fail initialization if another query cache implementation has been configured via node/index settings than the implementation a plugin wishes to use, is sufficient for now.

Member

martijnvg commented Aug 16, 2015

@uboness @rjernst I've updated the PR use ExtensionPoint to register query cache implementations.

I think the fact that a plugin can fail initialization if another query cache implementation has been configured via node/index settings than the implementation a plugin wishes to use, is sufficient for now.

@rjernst

This comment has been minimized.

Show comment
Hide comment
@rjernst

rjernst Aug 16, 2015

Member

Looks good, but can you add a test? See for example SearchModuleTests. Check a custom one can be registered and set, and that registering a duplicate name fails.

Member

rjernst commented Aug 16, 2015

Looks good, but can you add a test? See for example SearchModuleTests. Check a custom one can be registered and set, and that registering a duplicate name fails.

@martijnvg

This comment has been minimized.

Show comment
Hide comment
@martijnvg

martijnvg Aug 16, 2015

Member

@rjernst I've added a test.

Member

martijnvg commented Aug 16, 2015

@rjernst I've added a test.

@rjernst

This comment has been minimized.

Show comment
Hide comment
@rjernst

rjernst Aug 16, 2015

Member

LGTM, thanks!

Member

rjernst commented Aug 16, 2015

LGTM, thanks!

@s1monw

This comment has been minimized.

Show comment
Hide comment
@s1monw

s1monw Aug 17, 2015

Contributor

LGTM 2

Contributor

s1monw commented Aug 17, 2015

LGTM 2

@martijnvg martijnvg removed the review label Aug 17, 2015

martijnvg added a commit that referenced this pull request Aug 17, 2015

Merge pull request #12881 from martijnvg/allow_for_customable_query_c…
…ache

Allow a plugin to supply its own query cache implementation

@martijnvg martijnvg merged commit e649d96 into elastic:master Aug 17, 2015

1 check passed

CLA Commit author is a member of Elasticsearch
Details

martijnvg added a commit that referenced this pull request Aug 17, 2015

Merge pull request #12881 from martijnvg/allow_for_customable_query_c…
…ache

Allow a plugin to supply its own query cache implementation
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment