Allow InternalEngine to be stopped and started #8784
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Once the current engine is started you can only close it once. Once closed the engine cannot be started again. This commit adds a stop method which signals the engine to free it's resources but in a way that allows restarting. This is done by making InternalEngine a wrapper around InternalEngineImpl which maintains the start once, close once semantics. This allows to add the stop() method without adding complexity the engine implementation. InternalEngine is maintained as an entry point for listeners (incoming and outgoing) to other ES components, which abliviated the needs add/remove them if the engine is stopped.
| @@ -16,87 +16,33 @@ | |||
| * specific language governing permissions and limitations | |||
There was a problem hiding this comment.
I don't like the Foo / FooImpl pattern. I think the actual engine should be called InternalEngine and the configuration / holder part should be called InternalEngineDelegate or InternalEngineHolder or so?
There was a problem hiding this comment.
I didn't like the delegate naming because it's not really just a wrapper. I changed it to Holder now. The only down side I see is that the settings now have a different name (InternalEngineHolder#INDEX_FAIL_ON_MERGE_FAILURE and not InternalEngine#INDEX_FAIL_ON_MERGE_FAILURE). I think I'm good with this breaking change but pointing out.
|
left some minor naming comments - other than than LGTM |
|
@s1monw I pushed an update. Also note the breaking name in settings constants names. |
|
LGTM |
bleskes
added a commit
to bleskes/elasticsearch
that referenced
this pull request
Dec 8, 2014
Once the current engine is started you can only close it once. Once closed the engine cannot be started again. This commit adds a stop method which signals the engine to free it's resources but in a way that allows restarting. This is done by introducing InternalEngineHolder which is a wrapper around InternalEngine. This allows to add the stop() method without adding complexity the engine implementation. InternalEngineHolder also serves an entry point for listeners (incoming and outgoing) to other ES components, which removes the needs add/remove them if the engine is stopped. Closes elastic#8784
s1monw
added a commit
to s1monw/elasticsearch
that referenced
this pull request
Dec 10, 2014
After the refactoring in elastic#8784 some settings didn't get passed to the actual engine and there exists a race if the settings are updated while the engine is started such that the actual starting engine doesn't see the latest settings. This commit fixes the concurrency issue as well as adds tests to ensure the settings are reflected.
s1monw
added a commit
to s1monw/elasticsearch
that referenced
this pull request
Dec 10, 2014
After the refactoring in elastic#8784 some settings didn't get passed to the actual engine and there exists a race if the settings are updated while the engine is started such that the actual starting engine doesn't see the latest settings. This commit fixes the concurrency issue as well as adds tests to ensure the settings are reflected. Conflicts: src/main/java/org/elasticsearch/index/engine/internal/InternalEngine.java src/main/java/org/elasticsearch/index/engine/internal/InternalEngineHolder.java src/test/java/org/elasticsearch/index/engine/internal/InternalEngineTests.java
clintongormley
changed the title
clintongormley
added
:Distributed Indexing/Distributed
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Once the current engine is started you can only close it once. Once closed the engine cannot be started again. This commit adds a stop method which signals the engine to free it's resources but in a way that allows restarting.
This is done by introducing InternalEngineHolder which is a wrapper around InternalEngine. This allows to add the stop() method without adding complexity the engine implementation. InternalEngineHolder also serves an entry point for listeners (incoming and outgoing) to other ES components, which removes the needs add/remove them if the engine is stopped.
Needed for #8720