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

[ENGINE] Move engine lifecycle store reference to EngineHolder #8821

Merged
merged 1 commit into from Dec 8, 2014

Conversation

Projects
None yet
3 participants
@s1monw
Copy link
Contributor

commented Dec 8, 2014

This commit moves the engines reference to the store out of the actual
implementation into the hodler since the holder manages the actual lifcycle.
Engine internal references like per searcher or per recovery are kept inside
the actual implemenation since the have a different lifecycle.

this caused failures like this:

http://build-us-00.elasticsearch.org/job/es_core_master_metal/5917/

@s1monw s1monw added the review label Dec 8, 2014

@bleskes

View changes

src/test/java/org/elasticsearch/index/engine/internal/InternalEngineTests.java Outdated
@@ -1437,4 +1439,26 @@ public void testExtractShardId() {
assertEquals(shardId, engine.shardId());
};
}

public void testFailStart() throws IOException {
int refCount = store.refCount();

This comment has been minimized.

Copy link
@bleskes
}
} finally {
close(); // we need to close ourself - we failed all bets are off

This comment has been minimized.

Copy link
@bleskes

bleskes Dec 8, 2014

Member

can we add a comment to the PR description indicating this issue as well?

@s1monw

This comment has been minimized.

Copy link
Contributor Author

commented Dec 8, 2014

@bleskes I hardened this start method and made the store reference private to the engine. I also added an evil test that actually reproduces :)

@bleskes

This comment has been minimized.

Copy link
Member

commented Dec 8, 2014

@s1monw I like it. I left some comments on the commit. Sorry :(

@bleskes

This comment has been minimized.

Copy link
Member

commented Dec 8, 2014

LGTM

@s1monw s1monw force-pushed the s1monw:fix_internal_engine branch Dec 8, 2014

[ENGINE] Add engine lifecycle store reference to EngineHolder
This commit add the engines reference to the store out of the actual
implementation into the hodler since the holder manages the actual lifcycle.
Engine internal references like per searcher or per recovery are kept inside
the actual implemenation since the have a different lifecycle.

@s1monw s1monw force-pushed the s1monw:fix_internal_engine branch to b28fc1a Dec 8, 2014

@s1monw s1monw merged commit b28fc1a into elastic:master Dec 8, 2014

@clintongormley clintongormley removed the review label Mar 19, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.