[core] avoid stuck updateTaskStatus due to mulitple mesos updates #635
Conversation
This file contains hidden or 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
The issue described in OCTRL-953 occurs when we schedule kill of a task and while waiting for an acknowledgment, we receive two mesos updates: ``` [2024-11-11T12:14:02+01:00] TRACE scheduler: task status update received detector=TRG message= srcHost=alio2-cr1-flp163 state=TASK_FAILED task=2qwA9EYEgnY [2024-11-11T12:14:02+01:00] TRACE scheduler: task status update received detector=TRG message=Reconciliation: Task is unknown to the agent srcHost=alio2-cr1-flp163 state=TASK_LOST task=2qwA9EYEgnY ``` Which then trigger the discussed ack. Since it's inclear to me whether we can surely ignore TASK_LOST and trust that we will always receive either TASK_FAILED or TASK_FINISHED, I went for the approach of improving safeAcks to produce an error when subsequent acks are sent instead of blocking some goroutines.
Collaborator
Author
|
Tested on staging, it seems ok. |
justonedev1
reviewed
Nov 20, 2024
Collaborator
justonedev1
left a comment
justonedev1
left a comment
There was a problem hiding this comment.
it seems fine, I am just curious about the sync.Map
| // At the moment we utilize SafeAcks to acknowledge that all the requested tasks were killed by mesos (task/manager.go). | ||
| type SafeAcks struct { | ||
| mu sync.RWMutex | ||
| acks map[string]ackChannels |
Collaborator
There was a problem hiding this comment.
it seems to me that you are using mutex only to guard read/write into the map. Why not to use sync.Map?
Collaborator
Author
There was a problem hiding this comment.
I just wasn't aware it exists. However, I'm not sure if I could achieve the expected behaviour in RegisterAck with sync.Map as I need to do perform a read and write ideally in one mutex acquisition.
justonedev1
approved these changes
Nov 20, 2024
teo
approved these changes
Nov 25, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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 issue described in OCTRL-953 occurs when we schedule kill of a task and while waiting for an acknowledgment, we receive two mesos updates:
Which then trigger the discussed ack.
Since it's inclear to me whether we can surely ignore TASK_LOST and trust that we will always receive either TASK_FAILED or TASK_FINISHED, I went for the approach of improving safeAcks to produce an error when subsequent acks are sent instead of blocking some goroutines.