Fix fatal error: concurrent map writes for runtime controllers #3757
Conversation
Signed-off-by: trafalgarzzz <trafalgarz@outlook.com>
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #3757 +/- ##
=======================================
Coverage 64.19% 64.19%
=======================================
Files 474 474
Lines 28236 28236
=======================================
Hits 18127 18127
Misses 7945 7945
Partials 2164 2164 ☔ View full report in Codecov by Sentry. |
pkg/metrics/dataset_metrics.go
Outdated
| func init() { | ||
| metrics.Registry.MustRegister(datasetUFSFileNum, datasetUFSTotalSize) | ||
| datasetMetricsMap = map[string]*datasetMetrics{} | ||
| datasetMetricsMutex = &sync.Mutex{} |
There was a problem hiding this comment.
Why not using &sync.RWMutex{} to separate read and write?
There was a problem hiding this comment.
The two related operations are insertion and deletion. There is no read operation on metrics map.
There was a problem hiding this comment.
I see what you mean. RWMutex lgtm, will change it.
Signed-off-by: trafalgarzzz <trafalgarz@outlook.com>
Signed-off-by: trafalgarzzz <trafalgarz@outlook.com>
|
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: cheyang The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
…-cloudnative#3757) * Fix fatal error: concurrent map writes Signed-off-by: trafalgarzzz <trafalgarz@outlook.com> * Use RWMutex to separate read and write Signed-off-by: trafalgarzzz <trafalgarz@outlook.com> * Use sync.Map to protect metrics race condition Signed-off-by: trafalgarzzz <trafalgarz@outlook.com> --------- Signed-off-by: trafalgarzzz <trafalgarz@outlook.com>
Ⅰ. Describe what this PR does
#3756 is caused by concurrent writes on Fluid's metrics maps. Specifically, when there are multiple goroutines reconciling multiple Runtimes, it is possible that a deletion and a inserting happen at the same time on the same metrics map, which makes
fatal error: concurrent map writes.This PR fixes by adding Mutex to protect race condition on the maps.
Ⅱ. Does this pull request fix one issue?
fixes #3756
Ⅲ. List the added test cases (unit test/integration test) if any, please explain if no tests are needed.
Ⅳ. Describe how to verify it
Ⅴ. Special notes for reviews