From 471b0061b5c45eea5f31126fe2199493e1fc952f Mon Sep 17 00:00:00 2001 From: Massaki Archambault Date: Wed, 17 Jul 2024 23:35:18 -0400 Subject: [PATCH] guard gitlab cache Map with RWMutex to prevent concurrent read/write fixes #11 --- fstree/group.go | 2 +- fstree/refresh.go | 2 +- platforms/gitlab/client.go | 7 +++++-- platforms/gitlab/group.go | 20 +++++++++++++++++--- platforms/gitlab/user.go | 13 ++++++++++--- 5 files changed, 34 insertions(+), 10 deletions(-) diff --git a/fstree/group.go b/fstree/group.go index a820e62..a60337c 100644 --- a/fstree/group.go +++ b/fstree/group.go @@ -18,7 +18,7 @@ type groupNode struct { type GroupSource interface { GetGroupID() uint64 - InvalidateCache() + InvalidateContentCache() } // Ensure we are implementing the NodeReaddirer interface diff --git a/fstree/refresh.go b/fstree/refresh.go index 71cfdcc..ebf6fe5 100644 --- a/fstree/refresh.go +++ b/fstree/refresh.go @@ -41,6 +41,6 @@ func (n *refreshNode) Setattr(ctx context.Context, fh fs.FileHandle, in *fuse.Se } func (n *refreshNode) Open(ctx context.Context, flags uint32) (fh fs.FileHandle, fuseFlags uint32, errno syscall.Errno) { - n.source.InvalidateCache() + n.source.InvalidateContentCache() return nil, 0, 0 } diff --git a/platforms/gitlab/client.go b/platforms/gitlab/client.go index 7611ae4..fc885c5 100644 --- a/platforms/gitlab/client.go +++ b/platforms/gitlab/client.go @@ -4,6 +4,7 @@ import ( "fmt" "log/slog" "slices" + "sync" "github.com/badjware/gitlabfs/fstree" "github.com/xanzy/go-gitlab" @@ -34,8 +35,10 @@ type gitlabClient struct { currentUserCache *User // API response cache - groupCache map[int]*Group - userCache map[int]*User + groupCacheMux sync.RWMutex + groupCache map[int]*Group + userCacheMux sync.RWMutex + userCache map[int]*User } func NewClient(logger *slog.Logger, gitlabUrl string, gitlabToken string, p GitlabClientConfig) (*gitlabClient, error) { diff --git a/platforms/gitlab/group.go b/platforms/gitlab/group.go index 3ba961a..417f904 100644 --- a/platforms/gitlab/group.go +++ b/platforms/gitlab/group.go @@ -16,7 +16,7 @@ type Group struct { mux sync.Mutex - // group content + // hold group content childGroups map[string]fstree.GroupSource childProjects map[string]fstree.RepositorySource } @@ -25,15 +25,17 @@ func (g *Group) GetGroupID() uint64 { return uint64(g.ID) } -func (g *Group) InvalidateCache() { +func (g *Group) InvalidateContentCache() { g.mux.Lock() defer g.mux.Unlock() // clear child group from cache + g.gitlabClient.groupCacheMux.Lock() for _, childGroup := range g.childGroups { gid := int(childGroup.GetGroupID()) delete(g.gitlabClient.groupCache, gid) } + g.gitlabClient.groupCacheMux.Unlock() g.childGroups = nil // clear child repositories from cache @@ -43,7 +45,9 @@ func (g *Group) InvalidateCache() { func (c *gitlabClient) fetchGroup(gid int) (*Group, error) { // start by searching the cache // TODO: cache invalidation? + c.groupCacheMux.RLock() group, found := c.groupCache[gid] + c.groupCacheMux.RUnlock() if found { c.logger.Debug("Group cache hit", "gid", gid) return group, nil @@ -68,7 +72,9 @@ func (c *gitlabClient) fetchGroup(gid int) (*Group, error) { } // save in cache + c.groupCacheMux.Lock() c.groupCache[gid] = &newGroup + c.groupCacheMux.Unlock() return &newGroup, nil } @@ -77,15 +83,18 @@ func (c *gitlabClient) newGroupFromGitlabGroup(gitlabGroup *gitlab.Group) (*Grou gid := gitlabGroup.ID // start by searching the cache + c.groupCacheMux.RLock() group, found := c.groupCache[gid] + c.groupCacheMux.RUnlock() if found { + // if found in cache, return the cached reference c.logger.Debug("Group cache hit", "gid", gid) return group, nil } else { c.logger.Debug("Group cache miss; registering group", "gid", gid) } - // if not in cache, convert and save to cache now + // if not found in cache, convert and save to cache now newGroup := Group{ ID: gitlabGroup.ID, Name: gitlabGroup.Path, @@ -97,12 +106,17 @@ func (c *gitlabClient) newGroupFromGitlabGroup(gitlabGroup *gitlab.Group) (*Grou } // save in cache + c.groupCacheMux.Lock() c.groupCache[gid] = &newGroup + c.groupCacheMux.Unlock() return &newGroup, nil } func (c *gitlabClient) fetchGroupContent(group *Group) (map[string]fstree.GroupSource, map[string]fstree.RepositorySource, error) { + // Only a single routine can fetch the group content at the time. + // We lock for the whole duration of the function to avoid fetching the same data from the API + // multiple times if concurrent calls where to occur. group.mux.Lock() defer group.mux.Unlock() diff --git a/platforms/gitlab/user.go b/platforms/gitlab/user.go index fef1513..75acd9d 100644 --- a/platforms/gitlab/user.go +++ b/platforms/gitlab/user.go @@ -14,7 +14,7 @@ type User struct { mux sync.Mutex - // user content + // hold user content childProjects map[string]fstree.RepositorySource } @@ -22,7 +22,7 @@ func (u *User) GetGroupID() uint64 { return uint64(u.ID) } -func (u *User) InvalidateCache() { +func (u *User) InvalidateContentCache() { u.mux.Lock() defer u.mux.Unlock() @@ -33,15 +33,18 @@ func (u *User) InvalidateCache() { func (c *gitlabClient) fetchUser(uid int) (*User, error) { // start by searching the cache // TODO: cache invalidation? + c.userCacheMux.RLock() user, found := c.userCache[uid] + c.userCacheMux.RUnlock() if found { + // if found in cache, return the cached reference c.logger.Debug("User cache hit", "uid", uid) return user, nil } else { c.logger.Debug("User cache miss", "uid", uid) } - // If not in cache, fetch group infos from API + // If not found in cache, fetch group infos from API gitlabUser, _, err := c.client.Users.GetUser(uid) if err != nil { return nil, fmt.Errorf("failed to fetch user with id %v: %v", uid, err) @@ -55,6 +58,7 @@ func (c *gitlabClient) fetchUser(uid int) (*User, error) { // save in cache c.userCache[uid] = &newUser + c.userCacheMux.Unlock() return &newUser, nil } @@ -77,6 +81,9 @@ func (c *gitlabClient) fetchCurrentUser() (*User, error) { } func (c *gitlabClient) fetchUserContent(user *User) (map[string]fstree.GroupSource, map[string]fstree.RepositorySource, error) { + // Only a single routine can fetch the user content at the time. + // We lock for the whole duration of the function to avoid fetching the same data from the API + // multiple times if concurrent calls where to occur. user.mux.Lock() defer user.mux.Unlock()