Skip to content

Re-evaluate predicate in FnCondition.whenReadyIf as the predicate may… #502

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

Merged
merged 1 commit into from
Feb 13, 2023

Conversation

ginkel
Copy link
Contributor

@ginkel ginkel commented Jan 31, 2023

… have changed while awaiting the condition

During tests we noticed that the new BulkIngester sporadically attempts to submit empty bulk requests (containing zero operations) causing an exception. Digging through the code the most probable cause is that the predicate supplied to FnCondition.whenReadyIf may change during condition.awaitUninterruptibly() (which temporarily releases the lock), so the predicate needs to be re-evaluated after that call returns. This is what this PR implements.

@swallez
Copy link
Member

swallez commented Feb 13, 2023

Very good catch @ginkel! Concurrency is hard... Thanks a lot for the patch!

@swallez swallez merged commit 4c3e48a into elastic:main Feb 13, 2023
@swallez swallez added the Category: Bug Something isn't working label Feb 13, 2023
swallez pushed a commit that referenced this pull request Feb 13, 2023
swallez pushed a commit that referenced this pull request Feb 13, 2023
swallez added a commit that referenced this pull request Feb 13, 2023
… have changed while awaiting the condition (#502) (#511)

Co-authored-by: Thilo-Alexander Ginkel <[email protected]>
swallez added a commit that referenced this pull request Feb 13, 2023
… have changed while awaiting the condition (#502) (#510)

Co-authored-by: Thilo-Alexander Ginkel <[email protected]>
@aliariff aliariff mentioned this pull request Mar 22, 2023
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.

2 participants