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

Simplify watcher indexing listener. #52627

Merged

Conversation

martijnvg
Copy link
Member

@martijnvg martijnvg commented Feb 21, 2020

Add watcher to trigger server after index operation has succeeded,
instead of adding a watch to trigger service before
the actual index operation has performed on the shard level.

This logic is simpler to reason about in the case that a failure
does occur during the execution of an index operation on
the shard level.

Relates to #52453, but I'm not sure whether it fixes that test failure,
but it will make debugging easier.

Add watcher to trigger server after index operation has succeeded,
instead of adding a watch to trigger service before
the actual index operation has performed on the shard level.

This logic is simpler to reason about in the case that a failure
does occur during the execution of an index operation on
the shard level.

Relates to elastic#52453, but I think doesn't fix it, but makes it easier
to debug.
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-core-features (:Core/Features/Watcher)

@jakelandis jakelandis self-requested a review February 27, 2020 15:37
Copy link
Contributor

@jakelandis jakelandis left a comment

Choose a reason for hiding this comment

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

LGTM. I can think of any issues that would arise from moving pre->post and agree this is easier to reason about.

@martijnvg martijnvg marked this pull request as ready for review February 28, 2020 11:46
@martijnvg martijnvg merged commit e860c6e into elastic:master Mar 2, 2020
martijnvg added a commit to martijnvg/elasticsearch that referenced this pull request Mar 3, 2020
Add watcher to trigger server after index operation has succeeded,
instead of adding a watch to trigger service before
the actual index operation has performed on the shard level.

This logic is simpler to reason about in the case that a failure
does occur during the execution of an index operation on
the shard level.

Relates to elastic#52453, but I think doesn't fix it, but makes it easier
to debug.
martijnvg added a commit that referenced this pull request Mar 3, 2020
Backport: #52627

Add watcher to trigger server after index operation has succeeded,
instead of adding a watch to trigger service before
the actual index operation has performed on the shard level.

This logic is simpler to reason about in the case that a failure
does occur during the execution of an index operation on
the shard level.

Relates to #52453, but I think doesn't fix it, but makes it easier
to debug.
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

3 participants