Watcher add stopped listener (#43939) #45670
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
When Watcher is stopped and there are still outstanding watches running Watcher will report it self as stopped. In normal cases, this is not problematic. However, for integration tests Watcher is started and stopped between each test to help ensure a clean slate for each test. The tests are blocking only on the stopped state and make an implicit assumption that all watches are finished if the Watcher is stopped. This is an incorrect assumption since Stopped really means, "I will not accept any more watches". This can lead to un-predictable behavior in the tests such as message : "Watch is already queued in thread pool" and state: "not_executed_already_queued". This can also change the .watcher-history if watches linger between tests. This commit changes the semantics of a manual stopping watcher to now mean: "I will not accept any more watches AND all running watches are complete". There is now an intermediary step "Stopping" and callback to allow transition to a "Stopped" state when all Watches have completed. Additionally since this impacts how long the tests will block waiting for a "Stopped" state, the timeout has been increased. Related: elastic#42409
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
When Watcher is stopped and there are still outstanding watches running
Watcher will report it self as stopped. In normal cases, this is not problematic.
However, for integration tests Watcher is started and stopped between
each test to help ensure a clean slate for each test. The tests are blocking
only on the stopped state and make an implicit assumption that all watches are
finished if the Watcher is stopped. This is an incorrect assumption since
Stopped really means, "I will not accept any more watches". This can lead to
un-predictable behavior in the tests such as message : "Watch is already queued
in thread pool" and state: "not_executed_already_queued".
This can also change the .watcher-history if watches linger between tests.
This commit changes the semantics of a manual stopping watcher to now mean:
"I will not accept any more watches AND all running watches are complete".
There is now an intermediary step "Stopping" and callback to allow transition
to a "Stopped" state when all Watches have completed.
Additionally since this impacts how long the tests will block waiting for a
"Stopped" state, the timeout has been increased.
Related: #42409