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

Porting watcher over to BulkProcessor2 #94133

Merged
merged 14 commits into from
Mar 2, 2023

Conversation

masseyke
Copy link
Member

@masseyke masseyke commented Feb 24, 2023

In #91238 we rewrote BulkProcessor to avoid deadlock that had been seen in the IlmHistoryStore. This PR ports watcher over to the new BulkProcessor2 implementation. The only real change is that watcher history documents are now indexed asynchronously instead of in a blocking way, meaning that tests had to change to account for this.

@elasticsearchmachine
Copy link
Collaborator

Hi @masseyke, I've created a changelog YAML for you.

@masseyke
Copy link
Member Author

@elasticmachine run elasticsearch-ci/part-1

@masseyke masseyke marked this pull request as ready for review February 28, 2023 14:53
@elasticsearchmachine elasticsearchmachine added the Team:Data Management Meta label for data/management team label Feb 28, 2023
@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/es-data-management (Team:Data Management)

Copy link
Member

@dakrone dakrone left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for porting this over Keith!

@masseyke masseyke merged commit 463fbb9 into elastic:main Mar 2, 2023
@masseyke masseyke deleted the using-bulkprocessor2-in-watcher branch March 2, 2023 14:40
elasticsearchmachine pushed a commit that referenced this pull request Mar 2, 2023
…94282)

In #94133 we started loading watcher history asynchronously. I missed
updating DeleteWatchTests to wait for the history to be indexed. This
fixes that.
elasticsearchmachine pushed a commit that referenced this pull request Mar 2, 2023
In #94133 we started
loading watcher history asynchronously. I missed updating a test in
HistoryIntegrationTests to wait for the history to be indexed. This
fixes that.
saarikabhasi pushed a commit to saarikabhasi/elasticsearch that referenced this pull request Apr 10, 2023
In elastic#91238 we rewrote BulkProcessor to avoid deadlock that had been seen in the IlmHistoryStore.
This commit ports watcher over to the new BulkProcessor2 implementation. The only real change
is that watcher history documents are now indexed asynchronously instead of in a blocking way,
meaning that tests had to change to account for this.
saarikabhasi pushed a commit to saarikabhasi/elasticsearch that referenced this pull request Apr 10, 2023
…lastic#94282)

In elastic#94133 we started loading watcher history asynchronously. I missed
updating DeleteWatchTests to wait for the history to be indexed. This
fixes that.
saarikabhasi pushed a commit to saarikabhasi/elasticsearch that referenced this pull request Apr 10, 2023
…4286)

In elastic#94133 we started
loading watcher history asynchronously. I missed updating a test in
HistoryIntegrationTests to wait for the history to be indexed. This
fixes that.
elasticsearchmachine pushed a commit that referenced this pull request Apr 27, 2023
…em (#95642)

My theory is that the failure at #95077 was caused by #94133. It looks
like the test is failing because the watcher history index exists, but
not all shards have been allocated. Previously the bulk processor used
to load watcher history was blocking. So if the watch completed you were
guaranteed that the watcher history shards had been allocated because it
didn't return until the watcher history documents had been written. The
new bulk processor works asynchrnously, and doesn't block watcher. This
change waits until the watcher history indices are green (all shards
allocated) before querying them.

Closes #95077
ChrisHegarty pushed a commit that referenced this pull request Aug 9, 2023
In #94133 we started
loading watcher history asynchronously. I missed updating a test in
HistoryIntegrationTests to wait for the history to be indexed. This
fixes that.
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