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

Backport ShardRecoveryHandlerrefactorings to 1.x #8570

Merged
merged 4 commits into from Nov 20, 2014
Merged

Conversation

s1monw
Copy link
Contributor

@s1monw s1monw commented Nov 20, 2014

this is a backport of several commits and since it's a biggy I think we should do some review cycles here.

mappingUpdatedAction.updateMappingOnMaster(indexService.index().getName(), documentMapper, indexService.indexUUID(), listener);
}
try {
if (!updatedOnMaster.await(internalActionTimeout.millis(), TimeUnit.MILLISECONDS)) {
Copy link
Member

Choose a reason for hiding this comment

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

Should this await be cancelable?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it should I will fix seperately in master and backport then ok?

Copy link
Member

Choose a reason for hiding this comment

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

Sounds good

@dakrone
Copy link
Member

dakrone commented Nov 20, 2014

Left one question, other than that LGTM

dakrone and others added 4 commits November 20, 2014 14:38
Previously the bulk of our shard recovery code was in a 300-line
anonymous class in `RecoverySource`. This made it difficult to find and
more difficult to read.

This factors out that code into a `ShardRecoveryHandler` class, adding
javadocs for each function and phase of the recovery, as well as
comments explaining some of the more esoteric functions performed during
recovery.

It's hoped that this will help more people understand Elasticsearch's
recovery procedure.

No *major* functionality has changed, only typo corrections, some minor
allocation improvements and logging clarification changes.

Conflicts:
	src/main/java/org/elasticsearch/indices/recovery/RecoverySource.java
…gs when creating an index

Test used `indices.recovery.concurrent_streams` when creating an index but this is a node setting. Moved it to the node settings and added similar settings to speed up concurrent recoveries.

Also fixed a misleading log message in ShardRecoveryHandler when logging a remove corruption
Today recovery sources are not cancled if a shard is closed. The recovery target
is already cancled when shards are closed but we should also cleanup and cancel
the sources side since it holds on to shard locks / references until it's closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Distributed/Recovery Anything around constructing a new shard, either from a local or a remote source. >enhancement v1.5.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants