fix: throw instead of silently overwriting timers #8785
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Here we throw an exception when a timerId was generated that already exists in the timerJobMap. Throwing here will fail when scheduling a new timer or when resubmitting a recurring timer. This should never happen, but we have one heapdump where it looks like it did: #8776
lenaschoenburg
force-pushed
the
throw-on-conflicting-timers
branch
from
d18f485
to
842988d
Compare
|
I think we should catch this exception in case re-submitting a recurring timer fails so that we don't kill an actor thread when this happens |
romansmirnov
requested changes
Feb 18, 2022
There was a problem hiding this comment.
I think we should catch this exception in case re-submitting a recurring timer fails so that we don't kill an actor thread when this happens
But I think we already have implemented a failure handling? Basically, the IllegalStateException is thrown which will be caught in ActorJob#execute() which may cause to fail the actor but to my understanding, the actor thread will keep running?
So, to me, the question would be if it is okay that the respective actor fails and what would be the impact of it?
What do you think?
|
bors r+ |
ghost
pushed a commit
that referenced
this pull request
Feb 22, 2022
8785: fix: throw instead of silently overwriting timers r=oleschoenburg a=oleschoenburg ## Description Here we throw an exception when a timerId was generated that alreadyexists in the timerJobMap. Throwing here will fail when scheduling a new timer or when resubmitting a recurring timer. This should never happen, but we have one heapdump where it looks like it did: #8776 ## Related issues <!-- Which issues are closed by this PR or are related --> relates to #8776 Co-authored-by: Ole Schönburg <ole.schoenburg@gmail.com>
|
Build failed: |
|
bors retry |
ghost
pushed a commit
that referenced
this pull request
Feb 22, 2022
8785: fix: throw instead of silently overwriting timers r=oleschoenburg a=oleschoenburg ## Description Here we throw an exception when a timerId was generated that alreadyexists in the timerJobMap. Throwing here will fail when scheduling a new timer or when resubmitting a recurring timer. This should never happen, but we have one heapdump where it looks like it did: #8776 ## Related issues <!-- Which issues are closed by this PR or are related --> relates to #8776 Co-authored-by: Ole Schönburg <ole.schoenburg@gmail.com>
|
Build failed: |
|
bors retry |
ghost
pushed a commit
that referenced
this pull request
Feb 22, 2022
8785: fix: throw instead of silently overwriting timers r=oleschoenburg a=oleschoenburg ## Description Here we throw an exception when a timerId was generated that alreadyexists in the timerJobMap. Throwing here will fail when scheduling a new timer or when resubmitting a recurring timer. This should never happen, but we have one heapdump where it looks like it did: #8776 ## Related issues <!-- Which issues are closed by this PR or are related --> relates to #8776 Co-authored-by: Ole Schönburg <ole.schoenburg@gmail.com>
|
Build failed: |
|
bors retry |
ghost
merged commit 
|
Successfully created backport PR #8826 for |
|
Successfully created backport PR #8827 for |
ghost
pushed a commit
that referenced
this pull request
Feb 22, 2022
ghost
pushed a commit
that referenced
this pull request
Feb 22, 2022
Zelldon
added
Release: 1.2.11
version:1.3.5
This pull request was closed.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Here we throw an exception when a timerId was generated that alreadyexists in the timerJobMap.
Throwing here will fail when scheduling a new timer or when resubmitting a recurring timer.
This should never happen, but we have one heapdump where it looks like it did: #8776
Related issues
relates to #8776
Definition of Done
Not all items need to be done depending on the issue and the pull request.
Code changes:
backport stable/1.3) to the PR, in case that fails you need to create backports manually.Testing:
Documentation: