OCTRL-920 Fixes for stuck calibration runs #609
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
- "environment" is changed to "partition", so IL recognizes it correctly - we log the name of the attempted transition if it is delayed due to other transition already taking place - we log when we resume a delayed transition or teardown. - we log what was the state attempted to be updated when the task is not in roster
The goroutine for performing would never exit in case that the auto-stop would be invalidated. Consequently, it would be stuck indefinitely, surpassing the environment's lifetime. I don't think this was causing any trouble except of just leaking goroutines and obfuscating the state of the application. We also invalidate the auto-stop in case of going to ERROR, so we do not trigger a spurious transition attempt when it's pointless to do so.
Parallel attempts to kill tasks were found to be the primary cause for stuck auto-environments. In particular, it was due to channels in ackKilledTasks (and handling them) not expecting multiple listeners, so either one of the two kill acknowledgments would be stuck waiting for the acknowledgment to be received, or the other side, waiting for acknowledgment would never get it. It would cause KillTasks to be stuck indefinitely, which blocks the main auto-environment code-path.
justonedev1
reviewed
Sep 18, 2024
| log.WithField("taskIds", strings.Join(unkillable.GetTaskIds(), ", ")). | ||
| Debugf("some tasks cannot be physically killed (already dead?), will instead only be removed from roster") | ||
| Debugf("some tasks cannot be physically killed (already dead or being killed in another goroutine?), will instead only be removed from roster") |
There was a problem hiding this comment.
does this happen a lot? Shouldn't this be a Warning?
There was a problem hiding this comment.
i think this can be a somewhat valid situation if two kill requests happen subsequently, so I'm not sure if a warning is needed. But I don't know how often this may happen.
justonedev1
approved these changes
Sep 18, 2024
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.
The three commits fix what was observed as wrong while investigating the case with env
2pedNJwSjL8. The updated core was confirmed to work correctly in this test scenario.