[
https://issues.apache.org/jira/browse/YUNIKORN-2220?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17791829#comment-17791829
]
Craig Condit edited comment on YUNIKORN-2220 at 11/30/23 11:33 PM:
-------------------------------------------------------------------
I don't think this is safe. The event recorder actually modifies the object
passed to it, hence the need to defensively copy ahead of time. Since the
events process asyncronously, we can't rely on the object being private without
a clone.
In this case, there are two different events, so the object may be modified by
two different goroutines. It's not safe to assume they will not collide.
was (Author: ccondit):
I don't think this is safe. The event recorder actually modifies the object
passed to it, hence the need to defensively copy ahead of time. Since the
events process asyncronously, we can't rely on the object being private without
a clone.
> pod.DeepCopy() is called twice in Task
> --------------------------------------
>
> Key: YUNIKORN-2220
> URL: https://issues.apache.org/jira/browse/YUNIKORN-2220
> Project: Apache YuniKorn
> Issue Type: Improvement
> Components: shim - kubernetes
> Reporter: Peter Bacsko
> Assignee: Peter Bacsko
> Priority: Minor
>
> A small improvement is possible in {{task.go}}.
> In {{handleSubmitTaskEvent()}} and {{{}postTaskAllocated(){}}}, we call
> {{pod.DeepCopy()}} twice to avoid possible race conditions, but a single copy
> is enough. Once we have a copy, it's local to the method.
> {noformat}
> events.GetRecorder().Eventf(task.pod.DeepCopy(), nil, v1.EventTypeNormal,
> "Scheduling", "Scheduling",
> "%s is queued and waiting for allocation", task.alias)
> // if this task belongs to a task group, that means the app has gang
> scheduling enabled
> // in this case, post an event to indicate the task is being gang
> scheduled
> if !task.placeholder && task.taskGroupName != "" {
> events.GetRecorder().Eventf(task.pod.DeepCopy(), nil,
> v1.EventTypeNormal, "GangScheduling", "GangScheduling",
> "Pod belongs to the taskGroup %s, it will be scheduled
> as a gang member", task.taskGroupName) <-- second copy if GS is used
> }
> {noformat}
> {noformat}
> events.GetRecorder().Eventf(task.pod.DeepCopy(),
> nil, v1.EventTypeNormal, "Scheduled", "Scheduled",
> "Successfully assigned %s to node %s", task.alias, task.nodeName)
> ...
> events.GetRecorder().Eventf(task.pod.DeepCopy(), nil,
> v1.EventTypeNormal, "PodBindSuccessful", "PodBindSuccessful",
> "Pod %s is successfully bound to node %s", task.alias, task.nodeName)
> {noformat}
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
