-
Notifications
You must be signed in to change notification settings - Fork 813
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
added mutex at right places #3678
Conversation
Build Succeeded 👏 Build Id: 33c440c1-ecf6-4c03-b214-284132e8ae80 The following development artifacts have been built, and will exist for the next 30 days:
A preview of the website (the last 30 builds are retained): To install this version:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need to have similar locking here:
allocCnts[clientID]++ |
Looks like allocCnts
has the same issue.
When we get to
agones/test/load/allocation/runscenario/runscenario.go
Lines 146 to 152 in 646a1af
for j := 0; j < sc.numOfClients; j++ { | |
scnAllocCnt += allocCnts[j] | |
scnFailureCnt += failureCnts[j] | |
for k, v := range failureDtls[j] { | |
totalFailureDtls[k] += v | |
scnErrDtls[k] += v | |
} |
Also wondering if that should be locked?
646a1af
to
2e83d24
Compare
Yes, We can have the mutex here.
Ratio of error is very less here. We can have it without mutex, But, However rather than using mutiple mutex i though of utilizing |
Build Succeeded 👏 Build Id: eb14ceab-8970-44cb-b97d-f10e6cb58357 The following development artifacts have been built, and will exist for the next 30 days:
A preview of the website (the last 30 builds are retained): To install this version:
|
Clever! I like it. |
What type of PR is this?
What this PR does / Why we need it: This PR will resolve concurrent map writes issue.
Which issue(s) this PR fixes: #3670
Closes #3670
Special notes for your reviewer: