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

Avoid unneeded refresh with concurrent realtime gets #47895

Merged
merged 4 commits into from
Oct 13, 2019

Conversation

dnhatn
Copy link
Member

@dnhatn dnhatn commented Oct 11, 2019

This change should reduce refreshes for a use-case where we perform multiple realtime gets at the same time on an active index. Currently, we only call refresh if the index operation is still on the versionMap. However, at the time we call refresh, that operation might be already or will be included in the latest reader. Hence, we do not need to refresh. Adding another lock here is not an issue as the refresh is already sequential.

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

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

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

@ywelsch ywelsch added the v7.4.1 label Oct 11, 2019
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.

@dnhatn dnhatn added the v6.8.4 label Oct 11, 2019
@dnhatn
Copy link
Member Author

dnhatn commented Oct 12, 2019

The test failure is tracked at #43673

@dnhatn
Copy link
Member Author

dnhatn commented Oct 13, 2019

@ywelsch and @henningandersen thanks for reviewing.

@dnhatn dnhatn merged commit d8f5a3d into elastic:master Oct 13, 2019
@dnhatn dnhatn deleted the get-refresh branch October 13, 2019 16:48
dnhatn added a commit that referenced this pull request Oct 14, 2019
This change should reduce refreshes for a use-case where we perform 
multiple realtime gets at the same time on an active index. Currently,
we only call refresh if the index operation is still on the versionMap.
However, at the time we call refresh, that operation might be already or
will be included in the latest reader. Hence, we do not need to refresh.
Adding another lock here is not an issue as the refresh is already
sequential.
dnhatn added a commit that referenced this pull request Oct 14, 2019
This change should reduce refreshes for a use-case where we perform 
multiple realtime gets at the same time on an active index. Currently,
we only call refresh if the index operation is still on the versionMap.
However, at the time we call refresh, that operation might be already or
will be included in the latest reader. Hence, we do not need to refresh.
Adding another lock here is not an issue as the refresh is already
sequential.
dnhatn added a commit that referenced this pull request Oct 14, 2019
This change should reduce refreshes for a use-case where we perform
multiple realtime gets at the same time on an active index. Currently,
we only call refresh if the index operation is still on the versionMap.
However, at the time we call refresh, that operation might be already or
will be included in the latest reader. Hence, we do not need to refresh.
Adding another lock here is not an issue as the refresh is already
sequential.
howardhuanghua pushed a commit to TencentCloudES/elasticsearch that referenced this pull request Oct 14, 2019
This change should reduce refreshes for a use-case where we perform 
multiple realtime gets at the same time on an active index. Currently,
we only call refresh if the index operation is still on the versionMap.
However, at the time we call refresh, that operation might be already or
will be included in the latest reader. Hence, we do not need to refresh.
Adding another lock here is not an issue as the refresh is already
sequential.
dnhatn added a commit that referenced this pull request Oct 16, 2019
@dnhatn dnhatn added v6.8.4 and removed v6.8.4 labels Oct 16, 2019
dnhatn added a commit that referenced this pull request Oct 16, 2019
This change should reduce refreshes for a use-case where we perform
multiple realtime gets at the same time on an active index. Currently,
we only call refresh if the index operation is still on the versionMap.
However, at the time we call refresh, that operation might be already or
will be included in the latest reader. Hence, we do not need to refresh.
Adding another lock here is not an issue as the refresh is already
sequential.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Distributed Indexing/Engine Anything around managing Lucene and the Translog in an open shard. >enhancement v6.8.5 v7.4.1 v7.5.0 v8.0.0-alpha1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants