Fix race condition issue #391
Conversation
volcano-sh-bot
added
the
size/S
| @@ -253,6 +253,7 @@ func (cc *Controller) syncJob(jobInfo *apis.JobInfo, updateStatus state.UpdateSt | |||
|
|
|||
| waitCreationGroup := sync.WaitGroup{} | |||
| waitCreationGroup.Add(len(podToCreate)) | |||
| stateMutex := sync.Mutex{} | |||
There was a problem hiding this comment.
classifyAndAddUpPodBaseOnPhase is not the only place that we need to protect.
There was a problem hiding this comment.
the other also should move to function, do not put mutex in more than on place.
There was a problem hiding this comment.
FYI：
// A Mutex must not be copied after first use.
So this is not right, you can change to stateMutex := &sync.Mutex{}
There was a problem hiding this comment.
@hzxuzhonghu just curious, for which reason why this is not working? Which code leads to the multiple copies of mutex instance?
There was a problem hiding this comment.
Actually, this var is used by many go routines, and if you print the address of it in the go routines, you can find they are not same. So can not protect the critical-section using different lock.
There was a problem hiding this comment.
But the mutex instance is not used as a parameter for those go routines.
There was a problem hiding this comment.
make sense, my memory messed up.
TommyLike
force-pushed
the
bug/fix_race_issue
branch
from
07c5adb to
5c44771
Compare
|
Hey @TommyLike, TravisCI finished with status TravisBuddy Request Identifier: 07b62a00-b1d5-11e9-9ed2-8f218eb7efe2 |
|
/lgtm |
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: k82cn, TommyLike The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
volcano-sh-bot
added
the
approved
|
/lgtm |
|
@asifdxtreme what's the command to retest the patch? /retest |
For #359