search

kubernetes-sigs / cluster-api

Home for Cluster API, a subproject of sig-cluster-lifecycle
https://cluster-api.sigs.k8s.io
Apache License 2.0
3.5k stars 1.3k forks source link

Switch RWMutex's to sync.Map in remote cluster caches #2991

Closed JoelSpeed closed 4 years ago

JoelSpeed commented 4 years ago

User Story

This will improve the readability and maintainability of the remote cluster caching code

Detailed Description

The remote cluster caching code introduced in #2880 includes two maps that are accessed currently by grabbing a lock from two RWMutexs. These could be replaced by a sync.Map (or wrapper around it) that would then reduce this complexity within our code

(Originally suggested by @vincepri)

/kind feature

vincepri commented 4 years ago

/priority backlog

I'm fine keeping what we have today, this is more of a possible cleanup suggestion

vincepri commented 4 years ago

I think we can actually close this, reading the code again and discussing it more with @ncdc on Slack, I don't think sync.Map is useful in this case

/close

k8s-ci-robot commented 4 years ago

@vincepri: Closing this issue.

In response to [this](https://github.com/kubernetes-sigs/cluster-api/issues/2991#issuecomment-622432070): >I think we can actually close this, reading the code again and discussing it more with @ncdc on Slack, I don't think sync.Map is useful in this case > >/close Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.