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

Fix Watcher deadlock that can cause in-abilty to index documents. #41418

Merged
merged 9 commits into from
Apr 29, 2019

Conversation

jakelandis
Copy link
Contributor

This commit removes the usage of the BulkProcessor to write history documents
and delete triggered watches on a EsRejectedExecutionException. Since the
exception could be handled on the write thread, the write thread can be blocked
waiting on watcher threads (due to a synchronous method). This is problematic
since those watcher threads can be blocked waiting on write threads.

This commit also moves the handling of the exception to the generic threadpool
to avoid submitting write requests from the write thread pool.

fixes #41390

This commit removes the usage of the `BulkProcessor` to write history documents
and delete triggered watches on a `EsRejectedExecutionException`. Since the
exception could be handled on the write thread, the write thread can be blocked
waiting on watcher threads (due to a synchronous method). This is problematic
since those watcher threads can be blocked waiting on write threads.

This commit also moves the handling of the exception to the generic threadpool
to avoid submitting write requests from the write thread pool.

fixes elastic#41390
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-core-features

* Stores the specified watchRecord.
* Any existing watchRecord will be overwritten.
*/
private void forcePutHistory(WatchRecord watchRecord) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this method was pulled from git history, this is the implementation prior to the bulk processor.

Copy link
Member

@martijnvg martijnvg left a comment

Choose a reason for hiding this comment

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

In general this change looks good to me. However is it possible to write a integration test that simulates the dead lock this change is trying to fix?

@jaymode jaymode added v7.0.2 and removed v7.0.1 labels Apr 24, 2019
@jakelandis
Copy link
Contributor Author

Thanks for the review @martijnvg

I added an integration test that when executed against the old implementation will fail or deadlock , but works fine against this implementation.

@martijnvg martijnvg added the >bug label Apr 29, 2019
Copy link
Member

@martijnvg martijnvg left a comment

Choose a reason for hiding this comment

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

LGTM

@jakelandis jakelandis merged commit b8b5811 into elastic:master Apr 29, 2019
jakelandis added a commit to jakelandis/elasticsearch that referenced this pull request Apr 30, 2019
…astic#41418)

* Fix Watcher deadlock that can cause in-abilty to index documents.

This commit removes the usage of the `BulkProcessor` to write history documents
and delete triggered watches on a `EsRejectedExecutionException`. Since the
exception could be handled on the write thread, the write thread can be blocked
waiting on watcher threads (due to a synchronous method). This is problematic
since those watcher threads can be blocked waiting on write threads.

This commit also moves the handling of the exception to the generic threadpool
to avoid submitting write requests from the write thread pool.

fixes elastic#41390
jakelandis added a commit to jakelandis/elasticsearch that referenced this pull request Apr 30, 2019
…astic#41418)

* Fix Watcher deadlock that can cause in-abilty to index documents.

This commit removes the usage of the `BulkProcessor` to write history documents
and delete triggered watches on a `EsRejectedExecutionException`. Since the
exception could be handled on the write thread, the write thread can be blocked
waiting on watcher threads (due to a synchronous method). This is problematic
since those watcher threads can be blocked waiting on write threads.

This commit also moves the handling of the exception to the generic threadpool
to avoid submitting write requests from the write thread pool.

fixes elastic#41390
jakelandis added a commit to jakelandis/elasticsearch that referenced this pull request Apr 30, 2019
…astic#41418)

This commit removes the usage of the `BulkProcessor` to write history documents
and delete triggered watches on a `EsRejectedExecutionException`. Since the
exception could be handled on the write thread, the write thread can be blocked
waiting on watcher threads (due to a synchronous method). This is problematic
since those watcher threads can be blocked waiting on write threads.

This commit also moves the handling of the exception to the generic threadpool
to avoid submitting write requests from the write thread pool.

fixes elastic#41390
jakelandis added a commit that referenced this pull request Apr 30, 2019
…1418) (#41690)

This commit removes the usage of the `BulkProcessor` to write history documents
and delete triggered watches on a `EsRejectedExecutionException`. Since the
exception could be handled on the write thread, the write thread can be blocked
waiting on watcher threads (due to a synchronous method). This is problematic
since those watcher threads can be blocked waiting on write threads.

This commit also moves the handling of the exception to the generic threadpool
to avoid submitting write requests from the write thread pool.

fixes #41390

* includes changes to satisfy 6.x
jakelandis added a commit that referenced this pull request Apr 30, 2019
…1418) (#41685)

This commit removes the usage of the `BulkProcessor` to write history documents
and delete triggered watches on a `EsRejectedExecutionException`. Since the
exception could be handled on the write thread, the write thread can be blocked
waiting on watcher threads (due to a synchronous method). This is problematic
since those watcher threads can be blocked waiting on write threads.

This commit also moves the handling of the exception to the generic threadpool
to avoid submitting write requests from the write thread pool.

fixes #41390
jakelandis added a commit that referenced this pull request Apr 30, 2019
…1418) (#41684)

This commit removes the usage of the `BulkProcessor` to write history documents
and delete triggered watches on a `EsRejectedExecutionException`. Since the
exception could be handled on the write thread, the write thread can be blocked
waiting on watcher threads (due to a synchronous method). This is problematic
since those watcher threads can be blocked waiting on write threads.

This commit also moves the handling of the exception to the generic threadpool
to avoid submitting write requests from the write thread pool.

fixes #41390
akhil10x5 pushed a commit to akhil10x5/elasticsearch that referenced this pull request May 2, 2019
…astic#41418)

* Fix Watcher deadlock that can cause in-abilty to index documents.

This commit removes the usage of the `BulkProcessor` to write history documents
and delete triggered watches on a `EsRejectedExecutionException`. Since the
exception could be handled on the write thread, the write thread can be blocked
waiting on watcher threads (due to a synchronous method). This is problematic
since those watcher threads can be blocked waiting on write threads.

This commit also moves the handling of the exception to the generic threadpool
to avoid submitting write requests from the write thread pool.

fixes elastic#41390
@jakelandis jakelandis added v6.8.0 and removed v6.7.3 labels May 3, 2019
gurkankaymak pushed a commit to gurkankaymak/elasticsearch that referenced this pull request May 27, 2019
…astic#41418)

* Fix Watcher deadlock that can cause in-abilty to index documents.

This commit removes the usage of the `BulkProcessor` to write history documents
and delete triggered watches on a `EsRejectedExecutionException`. Since the
exception could be handled on the write thread, the write thread can be blocked
waiting on watcher threads (due to a synchronous method). This is problematic
since those watcher threads can be blocked waiting on write threads.

This commit also moves the handling of the exception to the generic threadpool
to avoid submitting write requests from the write thread pool.

fixes elastic#41390
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.

Watcher can deadlock resulting in-ability to index any documents.
4 participants