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

Add beforeIndexAddedToCluster callback #9514

Merged
merged 1 commit into from Jan 30, 2015

Conversation

dakrone
Copy link
Member

@dakrone dakrone commented Jan 30, 2015

This callback is executed only once, on the master node during an
index's creation. An exception thrown during this listener will cancel
the index creation.

This also adds checks in IndicesClusterStateService for the
indexService being null as well as if the indicesService.createIndex
throws an exception on data nodes after an index has already been
created.

@s1monw
Copy link
Contributor

s1monw commented Jan 30, 2015

LGTM

@dakrone dakrone force-pushed the lifecycle-cleanup branch 2 times, most recently from a1b3d37 to 9fe8406 Compare January 30, 2015 22:25
This callback is executed only once, on the master node during an
index's creation. An exception thrown during this listener will cancel
the index creation.

This also adds checks in `IndicesClusterStateService` for the
indexService being null as well as if the `indicesService.createIndex`
throws an exception on data nodes after an index has already been
created.
@dakrone dakrone merged commit 9fe8406 into elastic:master Jan 30, 2015
@@ -220,6 +220,10 @@ private void applyCleanedIndices(final ClusterChangedEvent event) {
// handle closed indices, since they are not allocated on a node once they are closed
// so applyDeletedIndices might not take them into account
for (IndexService indexService : indicesService) {
if (indexService == null) {
Copy link
Member

Choose a reason for hiding this comment

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

checking here for null is not benefetial I think, it can't be null...., same applies to other null checks while iterating

@kimchy
Copy link
Member

kimchy commented Jan 31, 2015

@dakrone I see it was merged, but at least the last comment I gave is critical

@dakrone
Copy link
Member Author

dakrone commented Feb 1, 2015

@kimchy I will fix that, thanks

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

Successfully merging this pull request may close these issues.

None yet

4 participants