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

Wait for mappings to be available on the primary before indexing. #10949

Merged
merged 1 commit into from May 6, 2015

Conversation

jpountz
Copy link
Contributor

@jpountz jpountz commented May 4, 2015

In some cases it might happen that a mapping which is already available on the
master node is not available yet on the node that holds the primary shard.
This commit changes indexing on the primary shard so that if a dynamic update
is triggered then the index operation is re-tried until required mappings are
available locally (using cluster state observing).

@jpountz jpountz added review v2.0.0-beta1 :Search/Mapping Index mappings, including merging and defining field types labels May 4, 2015
@@ -152,7 +165,8 @@ protected TransportRequestOptions transportOptions() {
}

protected boolean retryPrimaryException(Throwable e) {
return TransportActions.isShardNotAvailableException(e);
return e.getClass() == RetryOnPrimaryException.class
Copy link
Contributor

Choose a reason for hiding this comment

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

should we subclass IllegalIndexShardStateException instead of IndexShardException then we don't need to change retryPrimaryException

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK, I'll do that.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually IllegalIndexShardStateException seems to be about shards not being in the right shard state (STARTED, RECOVERING, ...) which does not seem to apply here

Copy link
Contributor

Choose a reason for hiding this comment

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

ok fair enough....

@s1monw
Copy link
Contributor

s1monw commented May 5, 2015

left 2 comments LGTM in general

@jpountz
Copy link
Contributor Author

jpountz commented May 6, 2015

@s1monw I initially liked the idea of reusing IllegalIndexShardStateException but this one is more about shards being eg. not started yet, while in this case the shard is started and working, it just happens to not have the latest version of the cluster state.

…exing.

In some cases it might happen that a mapping which is already available on the
master node is not available yet on the node that holds the primary shard.
This commit changes indexing on the primary shard so that if a dynamic update
is triggered then the index operation is re-tried until required mappings are
available locally (using cluster state observing).
@jpountz jpountz force-pushed the fix/wait_for_mappings_on_primary branch from 9917ef6 to 8a19bf3 Compare May 6, 2015 14:19
jpountz added a commit that referenced this pull request May 6, 2015
Mappings: Wait for mappings to be available on the primary before indexing.
@jpountz jpountz merged commit 19a6cb2 into elastic:master May 6, 2015
@kevinkluge kevinkluge removed the review label May 6, 2015
@jpountz jpountz deleted the fix/wait_for_mappings_on_primary branch May 6, 2015 14:22
@clintongormley clintongormley changed the title Mappings: Wait for mappings to be available on the primary before indexing. Wait for mappings to be available on the primary before indexing. Jun 8, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
>bug :Search/Mapping Index mappings, including merging and defining field types v2.0.0-beta1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants