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

Do not warm up searcher in engine constructor #48605

Merged
merged 4 commits into from
Oct 30, 2019

Conversation

dnhatn
Copy link
Member

@dnhatn dnhatn commented Oct 28, 2019

This PR is another prerequisite for #47186.

With this change, we won't warm up searchers until we externally refresh an engine. We explicitly refresh before allowing reading from a shard (i.e., move to post_recovery state) and during resetting. These guarantees that we have warmed up the engine before exposing the external searcher.

Relates #47186

@dnhatn dnhatn added >enhancement :Distributed/Engine Anything around managing Lucene and the Translog in an open shard. v8.0.0 v7.5.0 v7.6.0 labels Oct 28, 2019
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-distributed (:Distributed/Engine)

Copy link
Contributor

@jpountz jpountz left a comment

Choose a reason for hiding this comment

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

The change looks correct to me, but I'm not familiar enough with this part of the code to reason about potential implications, hopefully Henning or Yannick will be able to help here.

Copy link
Contributor

@ywelsch ywelsch left a comment

Choose a reason for hiding this comment

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

Left one comment, o.w. looking good

@dnhatn dnhatn requested a review from ywelsch October 29, 2019 17:05
Copy link
Contributor

@ywelsch ywelsch left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@henningandersen henningandersen left a comment

Choose a reason for hiding this comment

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

LGTM.

Left two minor comments to consider.

@dnhatn
Copy link
Member Author

dnhatn commented Oct 30, 2019

Thanks everyone for reviewing.

@dnhatn dnhatn merged commit 927cc34 into elastic:master Oct 30, 2019
@dnhatn dnhatn deleted the delay-warming branch October 30, 2019 18:09
dnhatn added a commit that referenced this pull request Oct 30, 2019
With this change, we won't warm up searchers until we externally refresh
an engine. We explicitly refresh before allowing reading from a shard
(i.e., move to post_recovery state) and during resetting. These
guarantees that we have warmed up the engine before exposing the
external searcher.

Another prerequisite for #47186.
dnhatn added a commit that referenced this pull request Nov 3, 2019
With this change, we won't warm up searchers until we externally refresh
an engine. We explicitly refresh before allowing reading from a shard
(i.e., move to post_recovery state) and during resetting. These
guarantees that we have warmed up the engine before exposing the
external searcher.

Another prerequisite for #47186.
dnhatn added a commit that referenced this pull request Nov 26, 2019
We do not guarantee that EngineTestCase#getDocIds is called after the 
engine has been externally refreshed. Hence, we trip an assertion
assertSearcherIsWarmedUp.

CI: https://gradle-enterprise.elastic.co/s/pm2at5qmfm2iu

Relates #48605
dnhatn added a commit that referenced this pull request Nov 26, 2019
We do not guarantee that EngineTestCase#getDocIds is called after the 
engine has been externally refreshed. Hence, we trip an assertion
assertSearcherIsWarmedUp.

CI: https://gradle-enterprise.elastic.co/s/pm2at5qmfm2iu

Relates #48605
dnhatn added a commit that referenced this pull request Nov 26, 2019
We do not guarantee that EngineTestCase#getDocIds is called after the 
engine has been externally refreshed. Hence, we trip an assertion
assertSearcherIsWarmedUp.

CI: https://gradle-enterprise.elastic.co/s/pm2at5qmfm2iu

Relates #48605
dnhatn added a commit that referenced this pull request Jan 29, 2020
We need to warm up the engine (i.e., perform an external refresh) before
accessing the external refresh. Note that we refresh externally before
allowing reading from a shard.

Relates #48605
Closes #51548
dnhatn added a commit that referenced this pull request Jan 29, 2020
We need to warm up the engine (i.e., perform an external refresh) before
accessing the external refresh. Note that we refresh externally before
allowing reading from a shard.

Relates #48605
Closes #51548
dnhatn added a commit that referenced this pull request Jan 29, 2020
We need to warm up the engine (i.e., perform an external refresh) before
accessing the external refresh. Note that we refresh externally before
allowing reading from a shard.

Relates #48605
Closes #51548
dnhatn added a commit that referenced this pull request Jan 29, 2020
We need to warm up the engine (i.e., perform an external refresh) before
accessing the external refresh. Note that we refresh externally before
allowing reading from a shard.

Relates #48605
Closes #51548
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Distributed/Engine Anything around managing Lucene and the Translog in an open shard. >enhancement v7.5.0 v7.6.0 v8.0.0-alpha1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants