Skip to content

Only load FST off heap if we are actually using mmaps for the term dictionary #43158

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
Jun 13, 2019
Merged

Only load FST off heap if we are actually using mmaps for the term dictionary #43158

merged 2 commits into from
Jun 13, 2019

Conversation

s1monw
Copy link
Contributor

@s1monw s1monw commented Jun 12, 2019

Given the significant performance impact that NIOFS has when term dicts are
loaded off-heap this change enforces FstLoadMode#AUTO that loads term dicts
off-heap only if the underlying index input indicates a memory map.

Relates to #43150

@s1monw
Only load FST off heap if we are actually using mmaps for the term di…
981a876 
…ctionary

Given the significant performance impact that NIOFS has when term dicts are
loaded off-heap this change enforces FstLoadMode#AUTO that loads term dicts
off heap only if the underlying index input indicates a memory map.

Relates to elastic#43150
@s1monw s1monw added >enhancement :Distributed Indexing/Engine Anything around managing Lucene and the Translog in an open shard. v8.0.0 v7.2.0 labels Jun 12, 2019
@s1monw s1monw requested a review from jpountz June 12, 2019 13:24
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-distributed

@s1monw s1monw added v7.3.0 and removed v7.2.0 labels Jun 12, 2019
@s1monw s1monw requested a review from jimczi June 12, 2019 18:45
jimczi
jimczi approved these changes Jun 12, 2019
View reviewed changes
Copy link
Contributor

@jimczi jimczi left a comment

Choose a reason for hiding this comment

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

LGTM

@s1monw s1monw merged commit 4de85c4 into elastic:master Jun 13, 2019
s1monw added a commit that referenced this pull request Jun 13, 2019
@s1monw
Only load FST off heap if we are actually using mmaps for the term di…
f70141c 
…ctionary (#43158)

Given the significant performance impact that NIOFS has when term dicts are
loaded off-heap this change enforces FstLoadMode#AUTO that loads term dicts
off heap only if the underlying index input indicates a memory map.

Relates to #43150
@ywelsch ywelsch mentioned this pull request Jan 22, 2020
Closed
tlrx added a commit to tlrx/elasticsearch that referenced this pull request Mar 18, 2020
@tlrx
Restore off-heap loading for term dictionary in ReadOnlyEngine
05b8859 
This is a partial restore of elastic#43158, following decision taken in elastic#51247
@tlrx tlrx mentioned this pull request Mar 18, 2020
Merged
tlrx added a commit that referenced this pull request Mar 18, 2020
@tlrx
Restore off-heap loading for term dictionary in ReadOnlyEngine (#53713)
e1096b9 
This is a partial restore of #43158, following decision taken in #51247

Closes #51247
tlrx added a commit that referenced this pull request Mar 18, 2020
@tlrx
Restore off-heap loading for term dictionary in ReadOnlyEngine (#53713)
6cc564d 
This is a partial restore of #43158, following decision taken in #51247

Closes #51247
@codebrain codebrain mentioned this pull request Apr 1, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jimczi jimczi jimczi approved these changes

@jpountz jpountz Awaiting requested review from jpountz

Assignees
No one assigned
Labels
:Distributed Indexing/Engine Anything around managing Lucene and the Translog in an open shard. >enhancement v7.3.0 v8.0.0-alpha1
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@s1monw @elasticmachine @jimczi @jakelandis