Skip to content
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

Fix concurrent access to the lock map #3985

Merged
merged 2 commits into from
Jun 16, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog/unreleased/jsoncs3-lock-congestion.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,4 +3,5 @@ Bugfix: Reduce jsoncs3 lock congestion
We changed the locking strategy in the jsoncs3 share manager to cause less lock
congestion increasing the performance in certain scenarios.

https://github.com/cs3org/reva/pull/3985
https://github.com/cs3org/reva/pull/3964
18 changes: 5 additions & 13 deletions pkg/share/manager/jsoncs3/providercache/providercache.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,7 @@ const tracerName = "providercache"

// Cache holds share information structured by provider and space
type Cache struct {
lockMapLock sync.Mutex
lockMap map[string]*sync.Mutex
lockMap sync.Map

Providers map[string]*Spaces

Expand Down Expand Up @@ -106,16 +105,9 @@ func (s *Shares) UnmarshalJSON(data []byte) error {

// LockSpace locks the cache for a given space and returns an unlock function
func (c *Cache) LockSpace(spaceID string) func() {
lock := c.lockMap[spaceID]
if lock == nil {
c.lockMapLock.Lock()
lock = c.lockMap[spaceID]
if lock == nil {
c.lockMap[spaceID] = &sync.Mutex{}
lock = c.lockMap[spaceID]
}
c.lockMapLock.Unlock()
}
v, _ := c.lockMap.LoadOrStore(spaceID, &sync.Mutex{})
lock := v.(*sync.Mutex)

lock.Lock()
return func() { lock.Unlock() }
}
Expand All @@ -126,7 +118,7 @@ func New(s metadata.Storage, ttl time.Duration) Cache {
Providers: map[string]*Spaces{},
storage: s,
ttl: ttl,
lockMap: map[string]*sync.Mutex{},
lockMap: sync.Map{},
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,7 @@ const tracerName = "receivedsharecache"
// It functions as an in-memory cache with a persistence layer
// The storage is sharded by user
type Cache struct {
lockMapLock sync.Mutex
lockMap map[string]*sync.Mutex
lockMap sync.Map

ReceivedSpaces map[string]*Spaces

Expand Down Expand Up @@ -79,21 +78,14 @@ func New(s metadata.Storage, ttl time.Duration) Cache {
ReceivedSpaces: map[string]*Spaces{},
storage: s,
ttl: ttl,
lockMap: map[string]*sync.Mutex{},
lockMap: sync.Map{},
}
}

func (c *Cache) lockUser(userID string) func() {
lock := c.lockMap[userID]
if lock == nil {
c.lockMapLock.Lock()
lock = c.lockMap[userID]
if lock == nil {
c.lockMap[userID] = &sync.Mutex{}
lock = c.lockMap[userID]
}
c.lockMapLock.Unlock()
}
v, _ := c.lockMap.LoadOrStore(userID, &sync.Mutex{})
lock := v.(*sync.Mutex)

lock.Lock()
return func() { lock.Unlock() }
}
Expand Down
18 changes: 5 additions & 13 deletions pkg/share/manager/jsoncs3/sharecache/sharecache.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,7 @@ const tracerName = "sharecache"
// It functions as an in-memory cache with a persistence layer
// The storage is sharded by user/group
type Cache struct {
lockMapLock sync.Mutex
lockMap map[string]*sync.Mutex
lockMap sync.Map

UserShares map[string]*UserShareCache

Expand All @@ -69,16 +68,9 @@ type SpaceShareIDs struct {
}

func (c *Cache) lockUser(userID string) func() {
lock := c.lockMap[userID]
if lock == nil {
c.lockMapLock.Lock()
lock = c.lockMap[userID]
if lock == nil {
c.lockMap[userID] = &sync.Mutex{}
lock = c.lockMap[userID]
}
c.lockMapLock.Unlock()
}
v, _ := c.lockMap.LoadOrStore(userID, &sync.Mutex{})
lock := v.(*sync.Mutex)

lock.Lock()
return func() { lock.Unlock() }
}
Expand All @@ -91,7 +83,7 @@ func New(s metadata.Storage, namespace, filename string, ttl time.Duration) Cach
namespace: namespace,
filename: filename,
ttl: ttl,
lockMap: map[string]*sync.Mutex{},
lockMap: sync.Map{},
}
}

Expand Down