Skip to content

Conversation

@s1monw
Copy link
Contributor

@s1monw s1monw commented Oct 20, 2015

Today we leak the notion of an engine outside of the shard abstraction
which is not desirable. This commit refactors the infrastrucutre to use
use already existing interfaces to communicate if a shard has failed and
prevents engine private classes to be implemented on a higher level.
This change is purely cosmentical...

Today we leak the notion of an engine outside of the shard abstraction
which is not desirable. This commit refactors the infrastrucutre to use
use already existing interfaces to communicate if a shard has failed and
prevents engine private classes to be implemented on a higher level.
This change is purely cosmentical...
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: since we're moving away from the word fail in the name, can we rename ShardEngineFailListener to ShardEngineEventListener

Copy link
Contributor Author

Choose a reason for hiding this comment

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

the shard end was not intended to be generic... I left that in here on purpose

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ah wrong I misunderstood... fixing

@bleskes
Copy link
Contributor

bleskes commented Oct 20, 2015

LGTM. Left one minor comment (next to trivial stuff) about null protection in IndicesClusterStateService

@s1monw
Copy link
Contributor Author

s1monw commented Oct 21, 2015

pushed a new commit adressing all comments and I also improved the situation if indexShard is null

Copy link
Contributor

Choose a reason for hiding this comment

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

thanks.

@bleskes
Copy link
Contributor

bleskes commented Oct 21, 2015

LGTM. Thx @s1monw

s1monw added a commit that referenced this pull request Oct 21, 2015
Refactor ShardFailure listener infrastructure
@s1monw s1monw merged commit 3ba4dfa into elastic:master Oct 21, 2015
@s1monw s1monw deleted the refactor_shard_failure_listener branch October 21, 2015 11:41
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.

3 participants