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

Pass down the EngineConfig to IndexSearcherWrapper #12883

Conversation

martijnvg
Copy link
Member

If a new IndexSearcher gets created in an IndexSearcherWrapper implementation then the engine config can be used to get the query cache and query cache policy from. Otherwise the new IndexSearcher being created uses the incorrect query cache and query cache policy.

* @param searcher The provided index searcher to be wrapped to add custom functionality
* @param searcher The provided index searcher to be wrapped to add custom functionality
* @param engineConfig The engine config which can be created to get the query cache and query cache policy from
* when creating a new index searcher
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't the @param engineConfig come first in the javadocs, since it's first in the args?

Copy link
Member Author

Choose a reason for hiding this comment

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

yes, it should, I'll change it.

@mikemccand
Copy link
Contributor

LGTM, I just left a minor comment about the javadocs...

If a new IndexSearcher gets created the engine config can be used to get the query cache and query cache policy from.
@martijnvg martijnvg force-pushed the index_seacher_wrapper_pass_down_engine_config branch from 0800815 to 2b97f5d Compare August 18, 2015 08:41
@martijnvg martijnvg merged commit 2b97f5d into elastic:master Aug 18, 2015
@jpountz
Copy link
Contributor

jpountz commented Aug 21, 2015

@martijnvg This is labeled as 2.0 but I can only see it in the master branch? Should it be backported or should we fix the version labels on this PR?

@martijnvg
Copy link
Member Author

@jpountz I think we should back port it. I'll do it once the beta release is out.

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