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.

Sign up for GitHub

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

Jump to bottom

fix(log/stream): ensure the appender future always gets completed #8605

Merged
1 commit merged into from
Jan 19, 2022
Merged

fix(log/stream): ensure the appender future always gets completed #8605

1 commit merged into from
Jan 19, 2022

Conversation

romansmirnov
Copy link
Member

@romansmirnov romansmirnov commented Jan 17, 2022 •
edited
Loading

Description

  • Handles any kind of thrown Throwables in the LogStream actor, so that the appender future gets completed exceptionally.
  • Handles the situation when opening the appender, the LogStream actor is supposed to be closed. In this situation, the appender future gets completed exceptionally as well.

Related issues

closes #7992

Definition of Done

Not all items need to be done depending on the issue and the pull request.

Code changes:

  • The changes are backwards compatibility with previous versions
  • If it fixes a bug then PRs are created to backport the fix to the last two minor versions. You can trigger a backport by assigning labels (e.g. backport stable/0.25) to the PR, in case that fails you need to create backports manually.

Testing:

  • There are unit/integration tests that verify all acceptance criterias of the issue
  • New tests are written to ensure backwards compatibility with further versions
  • The behavior is tested manually
  • The change has been verified by a QA run
  • The impact of the changes is verified by a benchmark

Documentation:

  • The documentation is updated (e.g. BPMN reference, configuration, examples, get-started guides, etc.)
  • New content is added to the release announcement

@romansmirnov romansmirnov force-pushed the 7992-log-stream-close-exceptionally branch from b36113a to ef887e6 Compare January 18, 2022 09:15
@romansmirnov romansmirnov marked this pull request as ready for review January 18, 2022 09:20
lenaschoenburg
lenaschoenburg approved these changes Jan 19, 2022
View reviewed changes
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.

Thanks @romansmirnov 👍

I have two minor 🔧 comments but nothing major that would require a re-review.

logstreams/src/main/java/io/camunda/zeebe/logstreams/impl/log/LogStreamImpl.java Outdated Show resolved Hide resolved
logstreams/src/main/java/io/camunda/zeebe/logstreams/impl/log/LogStreamImpl.java Show resolved Hide resolved
@romansmirnov romansmirnov force-pushed the 7992-log-stream-close-exceptionally branch from ef887e6 to a333f30 Compare January 19, 2022 12:56
@romansmirnov
Copy link
Member Author

bors r+

ghost pushed a commit that referenced this pull request Jan 19, 2022
@romansmirnov @dependabot
merge: #8582 #8605 #8615
a4112d8 
8582: fix(log/appender): yield thread when experiencing backpressure r=romansmirnov a=romansmirnov

## Description

Yield the thread, when the log storage appender experiences backpressure when trying to append the fragments to the log storage. That way, the actual actor task (log storage appender) is resubmitted to the working queue, and the actor thread is released to execute other actor tasks.

## Related issues

closes #8540 



8605: fix(log/stream): ensure the appender future always gets completed r=romansmirnov a=romansmirnov

## Description

* Handles any kind of thrown `Throwable`s in the `LogStream` actor, so that the appender future gets completed exceptionally.
* Handles the situation when opening the appender, the `LogStream` actor is supposed to be closed. In this situation, the appender future gets completed exceptionally as well.

## Related issues

closes #7992



8615: deps(maven): bump value from 2.8.9-ea-1 to 2.9.0 r=npepinpe a=dependabot[bot]

Bumps [value](https://github.com/immutables/immutables) from 2.8.9-ea-1 to 2.9.0.
<details>
<summary>Commits</summary>
<ul>
<li>See full diff in <a href="https://github.com/immutables/immutables/commits">compare view</a></li>
</ul>
</details>
<br />


[![Dependabot compatibility score](https://dependabot-badges.githubapp.com/badges/compatibility_score?dependency-name=org.immutables:value&package-manager=maven&previous-version=2.8.9-ea-1&new-version=2.9.0)](https://docs.github.com/en/github/managing-security-vulnerabilities/about-dependabot-security-updates#about-compatibility-scores)

Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting `@dependabot rebase`.

[//]: # (dependabot-automerge-start)
[//]: # (dependabot-automerge-end)

---

<details>
<summary>Dependabot commands and options</summary>
<br />

You can trigger Dependabot actions by commenting on this PR:
- `@dependabot rebase` will rebase this PR
- `@dependabot recreate` will recreate this PR, overwriting any edits that have been made to it
- `@dependabot merge` will merge this PR after your CI passes on it
- `@dependabot squash and merge` will squash and merge this PR after your CI passes on it
- `@dependabot cancel merge` will cancel a previously requested merge and block automerging
- `@dependabot reopen` will reopen this PR if it is closed
- `@dependabot close` will close this PR and stop Dependabot recreating it. You can achieve the same result by closing it manually
- `@dependabot ignore this major version` will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself)
- `@dependabot ignore this minor version` will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself)
- `@dependabot ignore this dependency` will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself)


</details>

Co-authored-by: Roman <roman.smirnov@camunda.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
@ghost
Copy link

ghost commented Jan 19, 2022

Build failed (retrying...):

@romansmirnov
Copy link
Member Author

bors retry

@ghost
Copy link

ghost commented Jan 19, 2022

Already running a review

@ghost
Copy link

ghost commented Jan 19, 2022

Build succeeded:

@ghost ghost merged commit 9e189a1 into develop Jan 19, 2022
@ghost ghost deleted the 7992-log-stream-close-exceptionally branch January 19, 2022 14:41
@github-actions
Copy link
Contributor

Backport failed for stable/1.2, because it was unable to cherry-pick the commit(s).

Please cherry-pick the changes locally.

git fetch origin stable/1.2
git worktree add -d .worktree/backport-8605-to-stable/1.2 origin/stable/1.2
cd .worktree/backport-8605-to-stable/1.2
git checkout -b backport-8605-to-stable/1.2
ancref=$(git merge-base 3da1a75d52e524b123f91a8ec9e480a0337bbf42 a333f30c916b5ab2e1cbdcc89c2b259d09e9d209)
git cherry-pick -x $ancref..a333f30c916b5ab2e1cbdcc89c2b259d09e9d209

@github-actions
Copy link
Contributor

Backport failed for stable/1.3, because it was unable to create a new branch.

Please cherry-pick the changes locally.

git fetch origin stable/1.3
git worktree add -d .worktree/backport-8605-to-stable/1.3 origin/stable/1.3
cd .worktree/backport-8605-to-stable/1.3
git checkout -b backport-8605-to-stable/1.3
ancref=$(git merge-base 3da1a75d52e524b123f91a8ec9e480a0337bbf42 a333f30c916b5ab2e1cbdcc89c2b259d09e9d209)
git cherry-pick -x $ancref..a333f30c916b5ab2e1cbdcc89c2b259d09e9d209

This was referenced Jan 21, 2022
Merged
Merged
ghost pushed a commit that referenced this pull request Jan 21, 2022
@romansmirnov
merge: #8628
67342f4 
8628: [Backport stable/1.3] fix(log/stream): ensure the appender future always gets completed r=oleschoenburg a=romansmirnov

## Description

<!-- Please explain the changes you made here. -->

## Related issues

<!-- Which issues are closed by this PR or are related -->

backports #8605
relates #7992



Co-authored-by: Roman <roman.smirnov@camunda.com>
ghost pushed a commit that referenced this pull request Jan 21, 2022
@romansmirnov
merge: #8628
da9674e 
8628: [Backport stable/1.3] fix(log/stream): ensure the appender future always gets completed r=oleschoenburg a=romansmirnov

## Description

<!-- Please explain the changes you made here. -->

## Related issues

<!-- Which issues are closed by this PR or are related -->

backports #8605
relates #7992



Co-authored-by: Roman <roman.smirnov@camunda.com>
ghost pushed a commit that referenced this pull request Jan 21, 2022
@romansmirnov
merge: #8627
2f1c74b 
8627: [Backport stable/1.2] fix(log/stream): ensure the appender future always gets completed r=oleschoenburg a=romansmirnov

## Description

<!-- Please explain the changes you made here. -->

## Related issues

<!-- Which issues are closed by this PR or are related -->

backports #8605 
relates #7992



Co-authored-by: Roman <roman.smirnov@camunda.com>
ghost pushed a commit that referenced this pull request Jan 25, 2022
@romansmirnov
merge: #8627
53fce69 
8627: [Backport stable/1.2] fix(log/stream): ensure the appender future always gets completed r=oleschoenburg a=romansmirnov

## Description

<!-- Please explain the changes you made here. -->

## Related issues

<!-- Which issues are closed by this PR or are related -->

backports #8605 
relates #7992



Co-authored-by: Roman <roman.smirnov@camunda.com>
@npepinpe npepinpe added version:1.3.2 Marks an issue as being completely or in parts released in 1.3.2 Release: 1.2.10 labels Jan 28, 2022
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lenaschoenburg lenaschoenburg lenaschoenburg approved these changes

Assignees
No one assigned
Labels
version:1.3.2 Marks an issue as being completely or in parts released in 1.3.2
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

OutOfMemory in follower
3 participants
@romansmirnov @lenaschoenburg @npepinpe