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

[Backport/stable 1.2] Fix ZeebePartition can be closed when there are ongoing transitions #8436

Merged
2 commits merged into from
Dec 20, 2021

Conversation

deepthidevaki
Copy link
Contributor

Backport of #8344

closes #7981

Due to merge conflicts, the commits that refactored the code are not backported.

…ebePartition

When `super.closeAsync` is called, it discards jobs from the Actor's queue. This resulted in an ongoing transition to get stuck, because the jobs related to it were discarded. Transition to inactive triggered by the close would then never completes, because it is waiting for the ongoing transition to complete. Thus ZeebePartition cannot be closed.
In this commit, we fix this behavior by ensuring that transitionToActive is called before actor close is requested. We also ensure that no new transitions can be enqueued, once the closing is started. This is important because we don't want new transition after it has all steps are closed by transitioning to inactive.

(cherry picked from commit 3a7cf18)
@deepthidevaki
Copy link
Contributor Author

@oleschoenburg There were some merge conflicts. So I did not backport some commits that contains only refactoring.

Copy link
Member

@lenaschoenburg lenaschoenburg 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've noticed that the fix for logging is not included, is that one of the "refactorings" that you intentionally didn't include? I think it'd be nice to have but I didn't check whether there are any conflicts in that area.

@deepthidevaki
Copy link
Contributor Author

I've noticed that the fix for logging is not included, is that one of the "refactorings" that you intentionally didn't include? I think it'd be nice to have but I didn't check whether there are any conflicts in that area.

Yes. That was intentionally left out, as there were lot of conflicts. Besides that code is not "on" by default in this version. It is the new transition logic which we disabled due to the bugs we found in 1.2.

@deepthidevaki
Copy link
Contributor Author

bors merge

@ghost
Copy link

ghost commented Dec 20, 2021

Build succeeded:

@ghost ghost merged commit 9e0278a into stable/1.2 Dec 20, 2021
@ghost ghost deleted the backport-8344-to-stable/1.2 branch December 20, 2021 17:34
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants