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

Add a frozen engine implementation #34357

Merged
merged 21 commits into from
Nov 7, 2018

Conversation

s1monw
Copy link
Contributor

@s1monw s1monw commented Oct 8, 2018

This change adds a frozen engine that allows lazily open a directory reader
on a read-only shard. The engine wraps general purpose searchers in a LazyDirectoryReader
that also allows to release and reset the underlying index readers after any and before
secondary search phases.

Relates to #34352

This change adds a `frozen` engine that allows lazily open a directory reader
on a read-only shard. The engine wraps general purpose searchers in a LazyDirectoryReader
that also allows to release and reset the underlying index readers after any and before
secondary search phases.

Relates to elastic#34352

F
@s1monw s1monw added >enhancement v7.0.0 :Distributed/Engine Anything around managing Lucene and the Translog in an open shard. labels Oct 8, 2018
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-distributed

@s1monw s1monw mentioned this pull request Oct 8, 2018
7 tasks
s1monw added a commit to s1monw/elasticsearch that referenced this pull request Oct 8, 2018
This change adds a high level freeze API that allows to open an
index frozen and vice versa. Indices must be closed in order to
become frozen and an open but frozen index must be closed to be
defrosted. This change also adds a `index.frozen` setting to
mark frozen indices and integrates the frozen engine with the
`SearchOperationListener` that resets and releases the directory
reader after and before search phases.

Relates to elastic#34352
Depends on elastic#34357
s1monw added a commit to s1monw/elasticsearch that referenced this pull request Oct 8, 2018
`Engine.Searcher` is non-final today which makes it error prone
in the case of wrapping the underlying reader or lucene `IndexSearcher`
like we do in `IndexSearcherWrapper`. Yet, there is no subclass of it yet
that would be dramtic to just drop on the floor. With the start of development
of frozen indices this changed since in elastic#34357 functionality was added to
a subclass which would be dropped if a `IndexSearcherWrapper` is installed on an index.
This change locks down the `Engine.Searcher` to prevent such a functionality trap.
s1monw added a commit that referenced this pull request Oct 16, 2018
`Engine.Searcher` is non-final today which makes it error prone
in the case of wrapping the underlying reader or lucene `IndexSearcher`
like we do in `IndexSearcherWrapper`. Yet, there is no subclass of it yet
that would be dramatic to just drop on the floor. With the start of development
of frozen indices this changed since in #34357 functionality was added to
a subclass which would be dropped if a `IndexSearcherWrapper` is installed on an index.
This change locks down the `Engine.Searcher` to prevent such a functionality trap.
s1monw added a commit that referenced this pull request Oct 16, 2018
`Engine.Searcher` is non-final today which makes it error prone
in the case of wrapping the underlying reader or lucene `IndexSearcher`
like we do in `IndexSearcherWrapper`. Yet, there is no subclass of it yet
that would be dramatic to just drop on the floor. With the start of development
of frozen indices this changed since in #34357 functionality was added to
a subclass which would be dropped if a `IndexSearcherWrapper` is installed on an index.
This change locks down the `Engine.Searcher` to prevent such a functionality trap.
Copy link
Contributor

@romseygeek romseygeek left a comment

Choose a reason for hiding this comment

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

I left a few comments. My only real question is where reset() and release() are going to be called from?


@Override
protected DirectoryReader open(Directory directory) throws IOException {
// we fake an empty directly reader for the ReadOnlyEngine. this reader is only used
Copy link
Contributor

Choose a reason for hiding this comment

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

s/directly/directory/

@s1monw
Copy link
Contributor Author

s1monw commented Oct 17, 2018

My only real question is where reset() and release() are going to be called from?

good one, This will be exposed in a followup. I do have a branch for this already but I didn't want to make this change too big. The relevant code is here

@romseygeek
Copy link
Contributor

TIL about SearchOperationListener. Nice.

kcm pushed a commit that referenced this pull request Oct 30, 2018
`Engine.Searcher` is non-final today which makes it error prone
in the case of wrapping the underlying reader or lucene `IndexSearcher`
like we do in `IndexSearcherWrapper`. Yet, there is no subclass of it yet
that would be dramatic to just drop on the floor. With the start of development
of frozen indices this changed since in #34357 functionality was added to
a subclass which would be dropped if a `IndexSearcherWrapper` is installed on an index.
This change locks down the `Engine.Searcher` to prevent such a functionality trap.
Copy link
Contributor

@bleskes bleskes left a comment

Choose a reason for hiding this comment

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

@s1monw as discussed - these were my comments and questions.

s1monw added a commit that referenced this pull request Nov 10, 2018
This change adds a high level freeze API that allows to mark an
index as frozen and vice versa. Indices must be closed in order to
become frozen and an open but frozen index must be closed to be
defrosted. This change also adds a index.frozen setting to
mark frozen indices and integrates the frozen engine with the
SearchOperationListener that resets and releases the directory
reader after and before search phases.

Relates to #34352
Depends on #34357
s1monw added a commit to s1monw/elasticsearch that referenced this pull request Nov 10, 2018
This change adds a special caching reader that caches all relevant
values for a range query to rewrite correctly in a can_match phase
without actually opening the underlying directory reader. This
allows frozen indices to be filtered with can_match and in-turn
searched with wildcards in a efficient way since it allows us to
exclude shards that won't match based on their date-ranges without
opening their directory readers.

Relates to elastic#34352
Depends on elastic#34357
pgomulka pushed a commit to pgomulka/elasticsearch that referenced this pull request Nov 13, 2018
This change adds a `frozen` engine that allows lazily open a directory reader
on a read-only shard. The engine wraps general purpose searchers in a LazyDirectoryReader
that also allows to release and reset the underlying index readers after any and before
secondary search phases.

Relates to elastic#34352
hendrikmuhs pushed a commit that referenced this pull request Nov 13, 2018
This change adds a special caching reader that caches all relevant
values for a range query to rewrite correctly in a can_match phase
without actually opening the underlying directory reader. This
allows frozen indices to be filtered with can_match and in-turn
searched with wildcards in a efficient way since it allows us to
exclude shards that won't match based on their date-ranges without
opening their directory readers.

Relates to #34352
Depends on #34357
s1monw added a commit that referenced this pull request Nov 13, 2018
This change adds a special caching reader that caches all relevant
values for a range query to rewrite correctly in a can_match phase
without actually opening the underlying directory reader. This
allows frozen indices to be filtered with can_match and in-turn
searched with wildcards in a efficient way since it allows us to
exclude shards that won't match based on their date-ranges without
opening their directory readers.

Relates to #34352
Depends on #34357
tlrx added a commit that referenced this pull request Dec 4, 2018
tlrx added a commit that referenced this pull request Jan 9, 2019
tlrx added a commit that referenced this pull request Jan 10, 2019
tlrx added a commit that referenced this pull request Jan 11, 2019
tlrx added a commit that referenced this pull request Jan 15, 2019
tlrx added a commit that referenced this pull request Jan 22, 2019
tlrx added a commit that referenced this pull request Jan 29, 2019
tlrx added a commit that referenced this pull request Jan 29, 2019
tlrx added a commit that referenced this pull request Jan 30, 2019
@jimczi jimczi added v7.0.0-beta1 and removed v7.0.0 labels Feb 7, 2019
tlrx added a commit to tlrx/elasticsearch that referenced this pull request Mar 1, 2019
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 v6.6.0 v7.0.0-beta1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants