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

Integrate translog recovery into Engine / InternalEngine #10452

Merged
merged 1 commit into from Apr 13, 2015

Conversation

Projects
None yet
5 participants
@s1monw
Contributor

s1monw commented Apr 7, 2015

Today the engine writes the transaction log itself as well as manages
all the commit / translog mapping internally. Yet, if an engine is closed
and reopend it doesn't replay it's translog or does anything to be consistent
with it's latest state again.
This change moves the transaction log replay code into the Engine / InternalEngine
and adds unittests for replaying and consistency.

@s1monw

This comment has been minimized.

Show comment
Hide comment
@s1monw

s1monw Apr 7, 2015

Contributor

@bleskes if you have a day or two to review ;)

Contributor

s1monw commented Apr 7, 2015

@bleskes if you have a day or two to review ;)

@s1monw s1monw added the blocker label Apr 7, 2015

this.searcherManager = manager;
this.mergeSchedulerFailureListener = new FailEngineOnMergeFailure();
this.mergeSchedulerListener = new MergeSchedulerListener();
this.mergeScheduler.addListener(mergeSchedulerListener);
this.mergeScheduler.addFailureListener(mergeSchedulerFailureListener);
final TranslogRecoveryPerformer transformer = engineConfig.getTranslogRecoveryPerformer();

This comment has been minimized.

@bleskes

bleskes Apr 13, 2015

Member

a transformer and performer. Quite a guy :)

@bleskes

bleskes Apr 13, 2015

Member

a transformer and performer. Quite a guy :)

This comment has been minimized.

@s1monw

s1monw Apr 13, 2015

Contributor

don't think I like that guy... once the MapperAnalyzer is gone we can fold it in!

@s1monw

s1monw Apr 13, 2015

Contributor

don't think I like that guy... once the MapperAnalyzer is gone we can fold it in!

return new Tuple<>(currentTranslogId, nextTranslogId);
}
// translog id is not in the metadata - fix this inconsistency some code relies on this and old indices might not have it.
writer.setCommitData(Collections.singletonMap(Translog.TRANSLOG_ID_KEY, Long.toString(nextTranslogId)));

This comment has been minimized.

@bleskes

bleskes Apr 13, 2015

Member

can we add an info, if not warn message here? I wonder how old the index needs to be for this to kick in. Do we know?

@bleskes

bleskes Apr 13, 2015

Member

can we add an info, if not warn message here? I wonder how old the index needs to be for this to kick in. Do we know?

This comment has been minimized.

@s1monw

s1monw Apr 13, 2015

Contributor

I don't know really... I mean it can even happen if there is no index present?

@s1monw

s1monw Apr 13, 2015

Contributor

I don't know really... I mean it can even happen if there is no index present?

engine.close();
engine = new InternalEngine(engine.config(), false); // we need to reuse the engine config unless the parser.mappingModified won't work
assertTrue(currentTranslogId + "<" + translog.currentId(), currentTranslogId < translog.currentId());

This comment has been minimized.

@bleskes

bleskes Apr 13, 2015

Member

we can use assertThat(translog.currentId(), isGreaterThan(currentTranslogId) and get nice messages automatically.

@bleskes

bleskes Apr 13, 2015

Member

we can use assertThat(translog.currentId(), isGreaterThan(currentTranslogId) and get nice messages automatically.

This comment has been minimized.

@s1monw

s1monw Apr 13, 2015

Contributor

I hate assertThat -1

@s1monw

s1monw Apr 13, 2015

Contributor

I hate assertThat -1

super(config, skipInitialTranslogRecovery);
}
private synchronized MockContext getMockContext() {

This comment has been minimized.

@bleskes

bleskes Apr 13, 2015

Member

question - why did we need to delay this from the constructor?

@bleskes

bleskes Apr 13, 2015

Member

question - why did we need to delay this from the constructor?

This comment has been minimized.

@s1monw

s1monw Apr 13, 2015

Contributor

oh man we get the searcher in teh ctor in the uppper class that requires to call getSearcher which needs the context...

@s1monw

s1monw Apr 13, 2015

Contributor

oh man we get the searcher in teh ctor in the uppper class that requires to call getSearcher which needs the context...

@bleskes

This comment has been minimized.

Show comment
Hide comment
@bleskes

bleskes Apr 13, 2015

Member

I like the change. Left a bunch of comments. Most of them minor, with the exception of the recovery state stats handling.

Member

bleskes commented Apr 13, 2015

I like the change. Left a bunch of comments. Most of them minor, with the exception of the recovery state stats handling.

@s1monw

This comment has been minimized.

Show comment
Hide comment
@s1monw

s1monw Apr 13, 2015

Contributor

pushed some updates

Contributor

s1monw commented Apr 13, 2015

pushed some updates

@s1monw

This comment has been minimized.

Show comment
Hide comment
@s1monw

s1monw Apr 13, 2015

Contributor

@bleskes pushed a new commit

Contributor

s1monw commented Apr 13, 2015

@bleskes pushed a new commit

@s1monw

This comment has been minimized.

Show comment
Hide comment
@s1monw

s1monw Apr 13, 2015

Contributor

@bleskes I pushed that stuff into the performer as you requested I hope it's worth all the added complexity

Contributor

s1monw commented Apr 13, 2015

@bleskes I pushed that stuff into the performer as you requested I hope it's worth all the added complexity

@bleskes

This comment has been minimized.

Show comment
Hide comment
@bleskes

bleskes Apr 13, 2015

Member

LGTM. Thx @s1monw

Member

bleskes commented Apr 13, 2015

LGTM. Thx @s1monw

s1monw added a commit to s1monw/elasticsearch that referenced this pull request Apr 13, 2015

[RECOVERY] Integrate translog recovery into Engine / InternalEngine
Today the engine writes the transaction log itself as well as manages
all the commit / translog mapping internally. Yet, if an engine is closed
and reopend it doesn't replay it's translog or does anything to be consistent
with it's latest state again.
This change moves the transaction log replay code into the Engine / InternalEngine
and adds unittests for replaying and consistency.

Closes #10452
[RECOVERY] Integrate translog recovery into Engine / InternalEngine
Today the engine writes the transaction log itself as well as manages
all the commit / translog mapping internally. Yet, if an engine is closed
and reopend it doesn't replay it's translog or does anything to be consistent
with it's latest state again.
This change moves the transaction log replay code into the Engine / InternalEngine
and adds unittests for replaying and consistency.

Closes #10452

@s1monw s1monw merged commit b1c9dfc into elastic:master Apr 13, 2015

1 check passed

CLA Commit author is a member of Elasticsearch
Details

@s1monw s1monw deleted the s1monw:recover_from_translog_in_engine branch Apr 14, 2015

@clintongormley clintongormley changed the title from [RECOVERY] Integrate translog recovery into Engine / InternalEngine to Integrate translog recovery into Engine / InternalEngine Jun 7, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment