From 9cd8e34cce92789965e12abdf476324fcae81d2f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marko=20Ga=C4=87e=C5=A1a?= Date: Mon, 24 Feb 2025 17:37:49 +0000 Subject: [PATCH] techdebt: [CODE-3209]: move caches to store package (#3449) * fix test * Merge remote-tracking branch 'origin/main' into mg/cache/move-caches-to-store * use pointers to types for evictor * gob decoder requires ptr * update pubsub namespace * add generic evictor, clear cache on space update * Merge remote-tracking branch 'origin/main' into mg/cache/move-caches-to-store * added the missing MarkChanged call in spaces soft delete * move caches to store package --- app/api/controller/githook/post_receive.go | 2 +- app/api/controller/migrate/update_state.go | 2 +- app/api/controller/repo/default_branch.go | 2 +- app/api/controller/repo/move.go | 4 +- app/api/controller/repo/soft_delete.go | 2 +- app/api/controller/repo/update.go | 2 +- app/api/controller/space/move.go | 2 +- app/api/controller/space/soft_delete.go | 2 + app/services/importer/repository.go | 4 +- app/services/migrate/pullreq.go | 2 +- app/services/refcache/common.go | 25 ----- app/services/refcache/repo_finder.go | 74 +++++------- app/services/refcache/space_cache.go | 105 ------------------ app/services/refcache/space_finder.go | 57 +++------- app/services/refcache/wire.go | 50 ++------- app/store/cache.go | 9 ++ app/store/cache/evictor.go | 79 +++++++++++++ .../repo_cache.go => store/cache/repo_id.go} | 62 ++++------- app/store/cache/repo_ref.go | 66 +++++++++++ app/store/cache/space_id.go | 57 ++++++++++ app/store/cache/{path.go => space_path.go} | 34 ++++-- app/store/cache/wire.go | 77 +++++++++++-- app/store/database/setup_test.go | 5 +- cache/ttl_cache.go | 6 + cmd/gitness/wire.go | 3 +- cmd/gitness/wire_gen.go | 17 +-- types/repo.go | 10 +- 27 files changed, 415 insertions(+), 345 deletions(-) delete mode 100644 app/services/refcache/common.go delete mode 100644 app/services/refcache/space_cache.go create mode 100644 app/store/cache/evictor.go rename app/{services/refcache/repo_cache.go => store/cache/repo_id.go} (52%) create mode 100644 app/store/cache/repo_ref.go create mode 100644 app/store/cache/space_id.go rename app/store/cache/{path.go => space_path.go} (64%) diff --git a/app/api/controller/githook/post_receive.go b/app/api/controller/githook/post_receive.go index 1c64dedee..333fa168a 100644 --- a/app/api/controller/githook/post_receive.go +++ b/app/api/controller/githook/post_receive.go @@ -339,7 +339,7 @@ func (c *Controller) handleEmptyRepoPush( return } - c.repoFinder.MarkChanged(ctx, repo.ID) + c.repoFinder.MarkChanged(ctx, repo.Core()) if repo.DefaultBranch != oldName { c.repoReporter.DefaultBranchUpdated(ctx, &repoevents.DefaultBranchUpdatedPayload{ diff --git a/app/api/controller/migrate/update_state.go b/app/api/controller/migrate/update_state.go index 5bb9404f8..77ff5fb54 100644 --- a/app/api/controller/migrate/update_state.go +++ b/app/api/controller/migrate/update_state.go @@ -63,7 +63,7 @@ func (c *Controller) UpdateRepoState( return nil, fmt.Errorf("failed to update the repo state: %w", err) } - c.repoFinder.MarkChanged(ctx, repo.ID) + c.repoFinder.MarkChanged(ctx, repo) return repoFull, nil } diff --git a/app/api/controller/repo/default_branch.go b/app/api/controller/repo/default_branch.go index a5554148f..0a3fbef19 100644 --- a/app/api/controller/repo/default_branch.go +++ b/app/api/controller/repo/default_branch.go @@ -103,7 +103,7 @@ func (c *Controller) UpdateDefaultBranch( return nil, fmt.Errorf("failed to update the repo default branch on db:%w", err) } - c.repoFinder.MarkChanged(ctx, repo.ID) + c.repoFinder.MarkChanged(ctx, repo) repoOutput, err := GetRepoOutput(ctx, c.publicAccess, repoFull) if err != nil { diff --git a/app/api/controller/repo/move.go b/app/api/controller/repo/move.go index 0a2fe5659..163dd7e5c 100644 --- a/app/api/controller/repo/move.go +++ b/app/api/controller/repo/move.go @@ -92,7 +92,7 @@ func (c *Controller) Move(ctx context.Context, return nil, fmt.Errorf("failed to update repo: %w", err) } - c.repoFinder.MarkChanged(ctx, repo.ID) + c.repoFinder.MarkChanged(ctx, repo.Core()) // set public access for the new repo path if err := c.publicAccess.Set(ctx, enum.PublicResourceTypeRepo, repo.Path, isPublic); err != nil { @@ -115,7 +115,7 @@ func (c *Controller) Move(ctx context.Context, ) } - c.repoFinder.MarkChanged(ctx, repo.ID) + c.repoFinder.MarkChanged(ctx, repo.Core()) // revert public access changes only after we successfully restored original path if dErr = c.publicAccess.Set(ctx, enum.PublicResourceTypeRepo, repo.Path, isPublic); dErr != nil { diff --git a/app/api/controller/repo/soft_delete.go b/app/api/controller/repo/soft_delete.go index 60c6d567b..af0b78dba 100644 --- a/app/api/controller/repo/soft_delete.go +++ b/app/api/controller/repo/soft_delete.go @@ -114,7 +114,7 @@ func (c *Controller) SoftDeleteNoAuth( return fmt.Errorf("failed to soft delete repo from db: %w", err) } - c.repoFinder.MarkChanged(ctx, repo.ID) + c.repoFinder.MarkChanged(ctx, repo.Core()) return nil } diff --git a/app/api/controller/repo/update.go b/app/api/controller/repo/update.go index 47add51d5..50ba164ae 100644 --- a/app/api/controller/repo/update.go +++ b/app/api/controller/repo/update.go @@ -123,7 +123,7 @@ func (c *Controller) Update(ctx context.Context, return nil, fmt.Errorf("failed to update the repo: %w", err) } - c.repoFinder.MarkChanged(ctx, repo.ID) + c.repoFinder.MarkChanged(ctx, repo.Core()) err = c.auditService.Log(ctx, session.Principal, diff --git a/app/api/controller/space/move.go b/app/api/controller/space/move.go index c99f42f35..02b2334de 100644 --- a/app/api/controller/space/move.go +++ b/app/api/controller/space/move.go @@ -134,7 +134,7 @@ func (c *Controller) moveInner( return fmt.Errorf("failed to update the space in the db: %w", err) } - c.spaceFinder.MarkChanged(ctx, space.ID) + c.spaceFinder.MarkChanged(ctx, space.Core()) return nil }) diff --git a/app/api/controller/space/soft_delete.go b/app/api/controller/space/soft_delete.go index 6091d043d..097da6797 100644 --- a/app/api/controller/space/soft_delete.go +++ b/app/api/controller/space/soft_delete.go @@ -69,6 +69,8 @@ func (c *Controller) SoftDeleteNoAuth( return nil, fmt.Errorf("failed to soft delete the space: %w", err) } + c.spaceFinder.MarkChanged(ctx, space.Core()) + return softDelRes, nil } diff --git a/app/services/importer/repository.go b/app/services/importer/repository.go index 789a54b92..ec1b0ffbe 100644 --- a/app/services/importer/repository.go +++ b/app/services/importer/repository.go @@ -314,7 +314,7 @@ func (r *Repository) Handle(ctx context.Context, data string, _ job.ProgressRepo return fmt.Errorf("failed to update repository prior to the import: %w", err) } - r.repoFinder.MarkChanged(ctx, repo.ID) + r.repoFinder.MarkChanged(ctx, repo.Core()) log.Info().Msg("sync repository") @@ -341,7 +341,7 @@ func (r *Repository) Handle(ctx context.Context, data string, _ job.ProgressRepo return fmt.Errorf("failed to update repository after import: %w", err) } - r.repoFinder.MarkChanged(ctx, repo.ID) + r.repoFinder.MarkChanged(ctx, repo.Core()) if input.Pipelines != PipelineOptionConvert { return nil // assumes the value is enum.PipelineOptionIgnore diff --git a/app/services/migrate/pullreq.go b/app/services/migrate/pullreq.go index a2151f56e..1b5aea449 100644 --- a/app/services/migrate/pullreq.go +++ b/app/services/migrate/pullreq.go @@ -238,7 +238,7 @@ func (migrate PullReq) Import( return nil, err } - migrate.repoFinder.MarkChanged(ctx, repo.ID) + migrate.repoFinder.MarkChanged(ctx, repo) return pullReqs, nil } diff --git a/app/services/refcache/common.go b/app/services/refcache/common.go deleted file mode 100644 index c865c99cc..000000000 --- a/app/services/refcache/common.go +++ /dev/null @@ -1,25 +0,0 @@ -// Copyright 2023 Harness, Inc. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package refcache - -import "time" - -const ( - pubsubNamespace = "refcache" - pubsubTopicRepoUpdate = "repo-update" - pubsubTopicSpaceUpdate = "space-update" -) - -const cacheDuration = 15 * time.Minute diff --git a/app/services/refcache/repo_finder.go b/app/services/refcache/repo_finder.go index 3aa3f1f15..e4d72381a 100644 --- a/app/services/refcache/repo_finder.go +++ b/app/services/refcache/repo_finder.go @@ -16,67 +16,41 @@ package refcache import ( "context" - "encoding/binary" "fmt" "strconv" "github.com/harness/gitness/app/paths" "github.com/harness/gitness/app/store" - "github.com/harness/gitness/pubsub" + "github.com/harness/gitness/app/store/cache" "github.com/harness/gitness/types" - - "github.com/rs/zerolog/log" ) type RepoFinder struct { - repoStore store.RepoStore - spaceRefCache SpaceRefCache - repoIDCache RepoIDCache - repoRefCache RepoRefCache - pubsub pubsub.PubSub + repoStore store.RepoStore + spacePathCache store.SpacePathCache + repoIDCache store.RepoIDCache + repoRefCache store.RepoRefCache + evictor cache.Evictor[*types.RepositoryCore] } func NewRepoFinder( repoStore store.RepoStore, - spaceRefCache SpaceRefCache, - repoIDCache RepoIDCache, - repoRefCache RepoRefCache, - bus pubsub.PubSub, + spacePathCache store.SpacePathCache, + repoIDCache store.RepoIDCache, + repoRefCache store.RepoRefCache, + evictor cache.Evictor[*types.RepositoryCore], ) RepoFinder { - r := RepoFinder{ - repoStore: repoStore, - spaceRefCache: spaceRefCache, - repoIDCache: repoIDCache, - repoRefCache: repoRefCache, - pubsub: bus, + return RepoFinder{ + repoStore: repoStore, + spacePathCache: spacePathCache, + repoIDCache: repoIDCache, + repoRefCache: repoRefCache, + evictor: evictor, } - - ctx := context.Background() - - _ = bus.Subscribe(ctx, pubsubTopicRepoUpdate, func(payload []byte) error { - repoID := int64(binary.LittleEndian.Uint64(payload)) - repo, err := r.repoIDCache.Get(ctx, repoID) - if err != nil { - log.Ctx(ctx).Warn().Err(err).Msg("repoFinder: pubsub subscriber: failed to get repo by ID from cache") - return err - } - - r.repoRefCache.Evict(ctx, RepoCacheKey{spaceID: repo.ParentID, repoIdentifier: repo.Identifier}) - r.repoIDCache.Evict(ctx, repoID) - - return nil - }, pubsub.WithChannelNamespace(pubsubNamespace)) - - return r } -func (r RepoFinder) MarkChanged(ctx context.Context, repoID int64) { - var buff [8]byte - binary.LittleEndian.PutUint64(buff[:], uint64(repoID)) - err := r.pubsub.Publish(ctx, pubsubTopicRepoUpdate, buff[:], pubsub.WithPublishNamespace(pubsubNamespace)) - if err != nil { - log.Ctx(ctx).Warn().Err(err).Msg("failed to publish repo update event") - } +func (r RepoFinder) MarkChanged(ctx context.Context, repoCore *types.RepositoryCore) { + r.evictor.Evict(ctx, repoCore) } func (r RepoFinder) FindByID(ctx context.Context, repoID int64) (*types.RepositoryCore, error) { @@ -86,17 +60,19 @@ func (r RepoFinder) FindByID(ctx context.Context, repoID int64) (*types.Reposito func (r RepoFinder) FindByRef(ctx context.Context, repoRef string) (*types.RepositoryCore, error) { repoID, err := strconv.ParseInt(repoRef, 10, 64) if err != nil || repoID <= 0 { - spacePath, repoIdentifier, err := paths.DisectLeaf(repoRef) + spaceRef, repoIdentifier, err := paths.DisectLeaf(repoRef) if err != nil { return nil, fmt.Errorf("failed to disect extract repo idenfifier from path: %w", err) } - spaceID, err := r.spaceRefCache.Get(ctx, spacePath) + spacePath, err := r.spacePathCache.Get(ctx, spaceRef) if err != nil { return nil, fmt.Errorf("failed to get space from cache: %w", err) } - repoID, err = r.repoRefCache.Get(ctx, RepoCacheKey{spaceID: spaceID, repoIdentifier: repoIdentifier}) + key := types.RepoCacheKey{SpaceID: spacePath.SpaceID, RepoIdentifier: repoIdentifier} + + repoID, err = r.repoRefCache.Get(ctx, key) if err != nil { return nil, fmt.Errorf("failed to get repository ID by space ID and repo identifier: %w", err) } @@ -126,12 +102,12 @@ func (r RepoFinder) FindDeletedByRef(ctx context.Context, repoRef string, delete return nil, fmt.Errorf("failed to disect extract repo idenfifier from path: %w", err) } - spaceID, err := r.spaceRefCache.Get(ctx, spaceRef) + spacePath, err := r.spacePathCache.Get(ctx, spaceRef) if err != nil { return nil, fmt.Errorf("failed to get space ID by space ref from cache: %w", err) } - repo, err := r.repoStore.FindDeletedByUID(ctx, spaceID, repoIdentifier, deleted) + repo, err := r.repoStore.FindDeletedByUID(ctx, spacePath.SpaceID, repoIdentifier, deleted) if err != nil { return nil, fmt.Errorf("failed to get deleted repository ID by space ID and repo identifier: %w", err) } diff --git a/app/services/refcache/space_cache.go b/app/services/refcache/space_cache.go deleted file mode 100644 index 5158dd7e0..000000000 --- a/app/services/refcache/space_cache.go +++ /dev/null @@ -1,105 +0,0 @@ -// Copyright 2023 Harness, Inc. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package refcache - -import ( - "context" - "fmt" - - "github.com/harness/gitness/app/paths" - "github.com/harness/gitness/app/store" - "github.com/harness/gitness/cache" - "github.com/harness/gitness/types" -) - -type ( - // SpaceIDCache holds the immutable part of Space objects fetched by space ID. - SpaceIDCache cache.Cache[int64, *types.SpaceCore] - - // SpaceRefCache holds the space ID fetched by space reference. - SpaceRefCache cache.Cache[string, int64] -) - -func NewSpaceIDCache( - spaceStore store.SpaceStore, -) SpaceIDCache { - return cache.New[int64, *types.SpaceCore](spaceIDCacheGetter{spaceStore: spaceStore}, cacheDuration) -} - -type spaceIDCacheGetter struct { - spaceStore store.SpaceStore -} - -func (g spaceIDCacheGetter) Find(ctx context.Context, spaceID int64) (*types.SpaceCore, error) { - space, err := g.spaceStore.Find(ctx, spaceID) - if err != nil { - return nil, fmt.Errorf("failed to find space by id: %w", err) - } - - return space.Core(), nil -} - -// spaceCache is a decorator of a Cache required to handle path transformations. -type spaceRefCache struct { - inner SpaceRefCache - spacePathTransformation store.SpacePathTransformation -} - -var _ cache.Cache[string, int64] = spaceRefCache{} - -func (c spaceRefCache) Get(ctx context.Context, spaceRef string) (int64, error) { - segments := paths.Segments(spaceRef) - uniqueKey := "" - for i, segment := range segments { - uniqueKey = paths.Concatenate(uniqueKey, c.spacePathTransformation(segment, i == 0)) - } - - return c.inner.Get(ctx, uniqueKey) -} - -func (c spaceRefCache) Stats() (int64, int64) { - return c.inner.Stats() -} - -func (c spaceRefCache) Evict(ctx context.Context, spaceRef string) { - c.inner.Evict(ctx, spaceRef) -} - -func NewSpaceRefCache( - spacePathStore store.SpacePathStore, - spacePathTransformation store.SpacePathTransformation, -) SpaceRefCache { - return &spaceRefCache{ - inner: cache.New[string, int64]( - pathToSpaceCacheGetter{ - spacePathStore: spacePathStore, - }, - cacheDuration), - spacePathTransformation: spacePathTransformation, - } -} - -type pathToSpaceCacheGetter struct { - spacePathStore store.SpacePathStore -} - -func (g pathToSpaceCacheGetter) Find(ctx context.Context, spaceRef string) (int64, error) { - path, err := g.spacePathStore.FindByPath(ctx, spaceRef) - if err != nil { - return 0, fmt.Errorf("failed to get space path by space ref: %w", err) - } - - return path.SpaceID, nil -} diff --git a/app/services/refcache/space_finder.go b/app/services/refcache/space_finder.go index 48f5c4154..164ad9eeb 100644 --- a/app/services/refcache/space_finder.go +++ b/app/services/refcache/space_finder.go @@ -16,59 +16,36 @@ package refcache import ( "context" - "encoding/binary" "fmt" "strconv" - "github.com/harness/gitness/pubsub" + "github.com/harness/gitness/app/store" + "github.com/harness/gitness/app/store/cache" "github.com/harness/gitness/types" - - "github.com/rs/zerolog/log" ) type SpaceFinder struct { - spaceIDCache SpaceIDCache - spaceRefCache SpaceRefCache - pubsub pubsub.PubSub + spaceIDCache store.SpaceIDCache + spacePathCache store.SpacePathCache + evictor cache.Evictor[*types.SpaceCore] } func NewSpaceFinder( - spaceIDCache SpaceIDCache, - spaceRefCache SpaceRefCache, - bus pubsub.PubSub, + spaceIDCache store.SpaceIDCache, + spacePathCache store.SpacePathCache, + evictor cache.Evictor[*types.SpaceCore], ) SpaceFinder { s := SpaceFinder{ - spaceIDCache: spaceIDCache, - spaceRefCache: spaceRefCache, - pubsub: bus, + spaceIDCache: spaceIDCache, + spacePathCache: spacePathCache, + evictor: evictor, } - ctx := context.Background() - - _ = bus.Subscribe(ctx, pubsubTopicSpaceUpdate, func(payload []byte) error { - spaceID := int64(binary.LittleEndian.Uint64(payload)) - space, err := s.spaceIDCache.Get(ctx, spaceID) - if err != nil { - log.Ctx(ctx).Warn().Err(err).Msg("spaceFinder: pubsub subscriber: failed to get space by ID from cache") - return err - } - - s.spaceRefCache.Evict(ctx, space.Path) - s.spaceIDCache.Evict(ctx, spaceID) - - return nil - }, pubsub.WithChannelNamespace(pubsubNamespace)) - return s } -func (s SpaceFinder) MarkChanged(ctx context.Context, spaceID int64) { - var buff [8]byte - binary.LittleEndian.PutUint64(buff[:], uint64(spaceID)) - err := s.pubsub.Publish(ctx, pubsubTopicSpaceUpdate, buff[:], pubsub.WithPublishNamespace(pubsubNamespace)) - if err != nil { - log.Ctx(ctx).Warn().Err(err).Msg("failed to publish space update event") - } +func (s SpaceFinder) MarkChanged(ctx context.Context, spaceCore *types.SpaceCore) { + s.evictor.Evict(ctx, spaceCore) } func (s SpaceFinder) FindByID(ctx context.Context, spaceID int64) (*types.SpaceCore, error) { @@ -80,13 +57,15 @@ func (s SpaceFinder) FindByID(ctx context.Context, spaceID int64) (*types.SpaceC return spaceCore, nil } -func (s SpaceFinder) FindByRef(ctx context.Context, spacePath string) (*types.SpaceCore, error) { - spaceID, err := strconv.ParseInt(spacePath, 10, 64) +func (s SpaceFinder) FindByRef(ctx context.Context, spaceRef string) (*types.SpaceCore, error) { + spaceID, err := strconv.ParseInt(spaceRef, 10, 64) if err != nil || spaceID <= 0 { - spaceID, err = s.spaceRefCache.Get(ctx, spacePath) + spacePath, err := s.spacePathCache.Get(ctx, spaceRef) if err != nil { return nil, fmt.Errorf("failed to get space ID by space path from cache: %w", err) } + + spaceID = spacePath.SpaceID } spaceCore, err := s.spaceIDCache.Get(ctx, spaceID) diff --git a/app/services/refcache/wire.go b/app/services/refcache/wire.go index 5781aafca..0d63cdd7a 100644 --- a/app/services/refcache/wire.go +++ b/app/services/refcache/wire.go @@ -16,60 +16,32 @@ package refcache import ( "github.com/harness/gitness/app/store" - "github.com/harness/gitness/pubsub" + "github.com/harness/gitness/app/store/cache" + "github.com/harness/gitness/types" "github.com/google/wire" ) // WireSet provides a wire set for this package. var WireSet = wire.NewSet( - ProvideSpaceIDCache, - ProvideSpaceRefCache, ProvideSpaceFinder, - ProvideRepoIDCache, - ProvideRepoRefCache, ProvideRepoFinder, ) -func ProvideSpaceIDCache( - spaceStore store.SpaceStore, -) SpaceIDCache { - return NewSpaceIDCache(spaceStore) -} - -func ProvideSpaceRefCache( - spacePathStore store.SpacePathStore, - pathTransform store.SpacePathTransformation, -) SpaceRefCache { - return NewSpaceRefCache(spacePathStore, pathTransform) -} - func ProvideSpaceFinder( - spaceIDCache SpaceIDCache, - spaceRefCache SpaceRefCache, - pubsub pubsub.PubSub, + spaceIDCache store.SpaceIDCache, + spaceRefCache store.SpacePathCache, + evictor cache.Evictor[*types.SpaceCore], ) SpaceFinder { - return NewSpaceFinder(spaceIDCache, spaceRefCache, pubsub) -} - -func ProvideRepoIDCache( - repoStore store.RepoStore, -) RepoIDCache { - return NewRepoIDCache(repoStore) -} - -func ProvideRepoRefCache( - repoStore store.RepoStore, -) RepoRefCache { - return NewRepoRefCache(repoStore) + return NewSpaceFinder(spaceIDCache, spaceRefCache, evictor) } func ProvideRepoFinder( repoStore store.RepoStore, - spaceRefCache SpaceRefCache, - repoIDCache RepoIDCache, - repoRefCache RepoRefCache, - pubsub pubsub.PubSub, + spaceRefCache store.SpacePathCache, + repoIDCache store.RepoIDCache, + repoRefCache store.RepoRefCache, + evictor cache.Evictor[*types.RepositoryCore], ) RepoFinder { - return NewRepoFinder(repoStore, spaceRefCache, repoIDCache, repoRefCache, pubsub) + return NewRepoFinder(repoStore, spaceRefCache, repoIDCache, repoRefCache, evictor) } diff --git a/app/store/cache.go b/app/store/cache.go index ff5cc4f3b..204f79504 100644 --- a/app/store/cache.go +++ b/app/store/cache.go @@ -23,9 +23,18 @@ type ( // PrincipalInfoCache caches principal IDs to principal info. PrincipalInfoCache cache.ExtendedCache[int64, *types.PrincipalInfo] + // SpaceIDCache holds the immutable part of Space objects fetched by space ID. + SpaceIDCache cache.Cache[int64, *types.SpaceCore] + // SpacePathCache caches a raw path to a space path. SpacePathCache cache.Cache[string, *types.SpacePath] + // RepoIDCache holds Repository objects fetched by their ID. + RepoIDCache cache.Cache[int64, *types.RepositoryCore] + + // RepoRefCache holds repository IDs fetched by spaceID and repository identifier. + RepoRefCache cache.Cache[types.RepoCacheKey, int64] + // InfraProviderResourceCache caches infraprovider resourceIDs to infraprovider resource. InfraProviderResourceCache cache.ExtendedCache[int64, *types.InfraProviderResource] ) diff --git a/app/store/cache/evictor.go b/app/store/cache/evictor.go new file mode 100644 index 000000000..5c95e9663 --- /dev/null +++ b/app/store/cache/evictor.go @@ -0,0 +1,79 @@ +// Copyright 2023 Harness, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package cache + +import ( + "bytes" + "context" + "encoding/gob" + + "github.com/harness/gitness/pubsub" + + "github.com/rs/zerolog/log" +) + +type Evictor[T any] struct { + nameSpace string + topicName string + bus pubsub.PubSub +} + +func NewEvictor[T any]( + nameSpace string, + topicName string, + bus pubsub.PubSub, +) Evictor[T] { + return Evictor[T]{ + nameSpace: nameSpace, + topicName: topicName, + bus: bus, + } +} + +func (e Evictor[T]) Subscribe(ctx context.Context, fn func(key T) error) { + if e.bus == nil { + return + } + + _ = e.bus.Subscribe(ctx, e.topicName, func(payload []byte) error { + var key T + err := gob.NewDecoder(bytes.NewReader(payload)).Decode(&key) + if err != nil { + log.Ctx(ctx).Warn().Err(err).Msgf("failed to process update event from type: %T", key) + return err + } + + return fn(key) + }, pubsub.WithChannelNamespace(e.nameSpace)) +} + +func (e Evictor[T]) Evict(ctx context.Context, key T) { + if e.bus == nil { + return + } + + buf := bytes.NewBuffer(nil) + _ = gob.NewEncoder(buf).Encode(key) + + err := e.bus.Publish( + ctx, + e.topicName, + buf.Bytes(), + pubsub.WithPublishNamespace(e.nameSpace), + ) + if err != nil { + log.Ctx(ctx).Warn().Err(err).Msgf("failed to publish update event for type %T", key) + } +} diff --git a/app/services/refcache/repo_cache.go b/app/store/cache/repo_id.go similarity index 52% rename from app/services/refcache/repo_cache.go rename to app/store/cache/repo_id.go index f766aa6fb..55d8f593c 100644 --- a/app/services/refcache/repo_cache.go +++ b/app/store/cache/repo_id.go @@ -12,36 +12,41 @@ // See the License for the specific language governing permissions and // limitations under the License. -package refcache +package cache import ( "context" "fmt" + "time" "github.com/harness/gitness/app/store" "github.com/harness/gitness/cache" "github.com/harness/gitness/types" ) -type ( - // RepoIDCache holds Repository objects fetched by their ID. - RepoIDCache cache.Cache[int64, *types.RepositoryCore] - - // RepoRefCache holds repository IDs fetched by spaceID and repository identifier. - RepoRefCache cache.Cache[RepoCacheKey, int64] -) - -type RepoCacheKey struct { - spaceID int64 - repoIdentifier string -} - func NewRepoIDCache( + appCtx context.Context, repoStore store.RepoStore, -) RepoIDCache { - return cache.New[int64, *types.RepositoryCore]( - repoIDCacheGetter{repoStore: repoStore}, - cacheDuration) + evictorSpace Evictor[*types.SpaceCore], + evictorRepo Evictor[*types.RepositoryCore], + dur time.Duration, +) store.RepoIDCache { + c := cache.New[int64, *types.RepositoryCore](repoIDCacheGetter{repoStore: repoStore}, dur) + + // In case when a space is updated, it's possible that a repo in the cache belongs the space or one of its parents. + // Rather than to dig through the cache to find if this is actually the case, it's simpler to clear the cache. + // Update of a space core (space identifier or space path) is a rare operation, so clearing cache is justified. + evictorSpace.Subscribe(appCtx, func(*types.SpaceCore) error { + c.EvictAll(appCtx) + return nil + }) + + evictorRepo.Subscribe(appCtx, func(repoCore *types.RepositoryCore) error { + c.Evict(appCtx, repoCore.ID) + return nil + }) + + return c } type repoIDCacheGetter struct { @@ -56,24 +61,3 @@ func (c repoIDCacheGetter) Find(ctx context.Context, repoID int64) (*types.Repos return repo.Core(), nil } - -func NewRepoRefCache( - repoStore store.RepoStore, -) RepoRefCache { - return cache.New[RepoCacheKey, int64]( - repoCacheGetter{repoStore: repoStore}, - cacheDuration) -} - -type repoCacheGetter struct { - repoStore store.RepoStore -} - -func (c repoCacheGetter) Find(ctx context.Context, repoKey RepoCacheKey) (int64, error) { - repo, err := c.repoStore.FindActiveByUID(ctx, repoKey.spaceID, repoKey.repoIdentifier) - if err != nil { - return 0, fmt.Errorf("failed to find repo by space ID and repo uid: %w", err) - } - - return repo.ID, nil -} diff --git a/app/store/cache/repo_ref.go b/app/store/cache/repo_ref.go new file mode 100644 index 000000000..8547b1228 --- /dev/null +++ b/app/store/cache/repo_ref.go @@ -0,0 +1,66 @@ +// Copyright 2023 Harness, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package cache + +import ( + "context" + "fmt" + "time" + + "github.com/harness/gitness/app/store" + "github.com/harness/gitness/cache" + "github.com/harness/gitness/types" +) + +func NewRepoRefCache( + appCtx context.Context, + repoStore store.RepoStore, + evictorSpace Evictor[*types.SpaceCore], + evictorRepo Evictor[*types.RepositoryCore], + dur time.Duration, +) store.RepoRefCache { + c := cache.New[types.RepoCacheKey, int64](repoCacheGetter{repoStore: repoStore}, dur) + + // In case when a space is updated, it's possible that a repo in the cache belongs the space or one of its parents. + // Rather than to dig through the cache to find if this is actually the case, it's simpler to clear the cache. + // Update of a space core (space identifier or space path) is a rare operation, so clearing cache is justified. + evictorSpace.Subscribe(appCtx, func(*types.SpaceCore) error { + c.EvictAll(appCtx) + return nil + }) + + evictorRepo.Subscribe(appCtx, func(repoCore *types.RepositoryCore) error { + c.Evict(appCtx, types.RepoCacheKey{ + SpaceID: repoCore.ParentID, + RepoIdentifier: repoCore.Identifier, + }) + return nil + }) + + return c +} + +type repoCacheGetter struct { + repoStore store.RepoStore +} + +func (c repoCacheGetter) Find(ctx context.Context, repoKey types.RepoCacheKey) (int64, error) { + repo, err := c.repoStore.FindActiveByUID(ctx, repoKey.SpaceID, repoKey.RepoIdentifier) + if err != nil { + return 0, fmt.Errorf("failed to find repo by space ID and repo uid: %w", err) + } + + return repo.ID, nil +} diff --git a/app/store/cache/space_id.go b/app/store/cache/space_id.go new file mode 100644 index 000000000..2fc1bb6b6 --- /dev/null +++ b/app/store/cache/space_id.go @@ -0,0 +1,57 @@ +// Copyright 2023 Harness, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package cache + +import ( + "context" + "fmt" + "time" + + "github.com/harness/gitness/app/store" + "github.com/harness/gitness/cache" + "github.com/harness/gitness/types" +) + +func NewSpaceIDCache( + appCtx context.Context, + spaceStore store.SpaceStore, + evictor Evictor[*types.SpaceCore], + dur time.Duration, +) store.SpaceIDCache { + c := cache.New[int64, *types.SpaceCore](spaceIDCacheGetter{spaceStore: spaceStore}, dur) + + // In case when a space is updated, we should remove from the cache the space and all of its subspaces. + // Rather than to dig through the cache to find all subspaces, it's simpler to clear the cache. + // Update of a space core (space identifier or space path) is a rare operation, so clearing cache is justified. + evictor.Subscribe(appCtx, func(*types.SpaceCore) error { + c.EvictAll(appCtx) + return nil + }) + + return c +} + +type spaceIDCacheGetter struct { + spaceStore store.SpaceStore +} + +func (g spaceIDCacheGetter) Find(ctx context.Context, spaceID int64) (*types.SpaceCore, error) { + space, err := g.spaceStore.Find(ctx, spaceID) + if err != nil { + return nil, fmt.Errorf("failed to find space by id: %w", err) + } + + return space.Core(), nil +} diff --git a/app/store/cache/path.go b/app/store/cache/space_path.go similarity index 64% rename from app/store/cache/path.go rename to app/store/cache/space_path.go index 6e04138d4..7b96628e4 100644 --- a/app/store/cache/path.go +++ b/app/store/cache/space_path.go @@ -25,23 +25,33 @@ import ( ) // pathCacheGetter is used to hook a spacePathStore as source of a PathCache. -// IMPORTANT: It assumes that the pathCache already transformed the key. +// IMPORTANT: It assumes that the spacePathCache already transformed the key. type pathCacheGetter struct { spacePathStore store.SpacePathStore } func New( + appCtx context.Context, pathStore store.SpacePathStore, spacePathTransformation store.SpacePathTransformation, + evictor Evictor[*types.SpaceCore], + dur time.Duration, ) store.SpacePathCache { - return &pathCache{ - inner: cache.New[string, *types.SpacePath]( - &pathCacheGetter{ - spacePathStore: pathStore, - }, - 1*time.Minute), + innerCache := cache.New[string, *types.SpacePath](&pathCacheGetter{spacePathStore: pathStore}, dur) + + c := spacePathCache{ + inner: innerCache, spacePathTransformation: spacePathTransformation, } + + // In case when a space core is updated, we should remove from the cache its space path and all of its sub-paths. + // Update of a space core (space identifier or space path) is a rare operation, so clearing cache is justified. + evictor.Subscribe(appCtx, func(*types.SpaceCore) error { + innerCache.EvictAll(appCtx) + return nil + }) + + return c } func (g *pathCacheGetter) Find(ctx context.Context, key string) (*types.SpacePath, error) { @@ -53,13 +63,13 @@ func (g *pathCacheGetter) Find(ctx context.Context, key string) (*types.SpacePat return path, nil } -// pathCache is a decorator of a Cache required to handle path transformations. -type pathCache struct { +// spacePathCache is a decorator of a Cache required to handle path transformations. +type spacePathCache struct { inner cache.Cache[string, *types.SpacePath] spacePathTransformation store.SpacePathTransformation } -func (c *pathCache) Get(ctx context.Context, key string) (*types.SpacePath, error) { +func (c spacePathCache) Get(ctx context.Context, key string) (*types.SpacePath, error) { // build unique key from provided value segments := paths.Segments(key) uniqueKey := "" @@ -70,10 +80,10 @@ func (c *pathCache) Get(ctx context.Context, key string) (*types.SpacePath, erro return c.inner.Get(ctx, uniqueKey) } -func (c *pathCache) Stats() (int64, int64) { +func (c spacePathCache) Stats() (int64, int64) { return c.inner.Stats() } -func (c *pathCache) Evict(ctx context.Context, key string) { +func (c spacePathCache) Evict(ctx context.Context, key string) { c.inner.Evict(ctx, key) } diff --git a/app/store/cache/wire.go b/app/store/cache/wire.go index b32d254bd..b44b3d7c6 100644 --- a/app/store/cache/wire.go +++ b/app/store/cache/wire.go @@ -15,33 +15,92 @@ package cache import ( + "context" "time" "github.com/harness/gitness/app/store" "github.com/harness/gitness/cache" + "github.com/harness/gitness/pubsub" "github.com/harness/gitness/types" "github.com/google/wire" ) -// WireSet provides a wire set for this package. -var WireSet = wire.NewSet( +// WireSetSpace provides a wire set for this package. +var WireSetSpace = wire.NewSet( ProvidePrincipalInfoCache, - ProvidePathCache, + ProvideEvictorSpaceCore, + ProvideSpaceIDCache, + ProvideSpacePathCache, ProvideInfraProviderResourceCache, ) -// ProvidePrincipalInfoCache provides a cache for storing types.PrincipalInfo objects. -func ProvidePrincipalInfoCache(getter store.PrincipalInfoView) store.PrincipalInfoCache { - return cache.NewExtended[int64, *types.PrincipalInfo](getter, 30*time.Second) +// WireSetRepo provides a repository related wire set for this package. +var WireSetRepo = wire.NewSet( + ProvideEvictorRepositoryCore, + ProvideRepoIDCache, + ProvideRepoRefCache, +) + +const ( + principalInfoCacheDuration = 30 * time.Second + spaceCacheDuration = 15 * time.Minute + repositoryCacheDuration = 15 * time.Minute +) + +const ( + pubsubNamespace = "cache-evictor" + pubsubTopicSpaceCoreUpdate = "space-core-update" + pubsubTopicRepoCoreUpdate = "repo-core-update" +) + +func ProvideEvictorSpaceCore(pubsub pubsub.PubSub) Evictor[*types.SpaceCore] { + return NewEvictor[*types.SpaceCore](pubsubNamespace, pubsubTopicSpaceCoreUpdate, pubsub) } -// ProvidePathCache provides a cache for storing routing paths and their types.SpacePath objects. -func ProvidePathCache( +func ProvideEvictorRepositoryCore(pubsub pubsub.PubSub) Evictor[*types.RepositoryCore] { + return NewEvictor[*types.RepositoryCore](pubsubNamespace, pubsubTopicRepoCoreUpdate, pubsub) +} + +// ProvidePrincipalInfoCache provides a cache for storing types.PrincipalInfo objects. +func ProvidePrincipalInfoCache(getter store.PrincipalInfoView) store.PrincipalInfoCache { + return cache.NewExtended[int64, *types.PrincipalInfo](getter, principalInfoCacheDuration) +} + +func ProvideSpaceIDCache( + appCtx context.Context, + spaceStore store.SpaceStore, + evictor Evictor[*types.SpaceCore], +) store.SpaceIDCache { + return NewSpaceIDCache(appCtx, spaceStore, evictor, spaceCacheDuration) +} + +// ProvideSpacePathCache provides a cache for storing routing paths and their types.SpacePath objects. +func ProvideSpacePathCache( + appCtx context.Context, pathStore store.SpacePathStore, + evictor Evictor[*types.SpaceCore], spacePathTransformation store.SpacePathTransformation, ) store.SpacePathCache { - return New(pathStore, spacePathTransformation) + return New(appCtx, pathStore, spacePathTransformation, evictor, spaceCacheDuration) +} + +func ProvideRepoIDCache( + appCtx context.Context, + repoStore store.RepoStore, + evictorSpace Evictor[*types.SpaceCore], + evictorRepo Evictor[*types.RepositoryCore], +) store.RepoIDCache { + return NewRepoIDCache(appCtx, repoStore, evictorSpace, evictorRepo, repositoryCacheDuration) +} + +func ProvideRepoRefCache( + appCtx context.Context, + repoStore store.RepoStore, + evictorSpace Evictor[*types.SpaceCore], + evictorRepo Evictor[*types.RepositoryCore], +) store.RepoRefCache { + return NewRepoRefCache(appCtx, repoStore, evictorSpace, evictorRepo, repositoryCacheDuration) } // ProvideInfraProviderResourceCache provides a cache for storing types.InfraProviderResource objects. diff --git a/app/store/database/setup_test.go b/app/store/database/setup_test.go index 3c1baca8b..5f82bdb41 100644 --- a/app/store/database/setup_test.go +++ b/app/store/database/setup_test.go @@ -20,6 +20,7 @@ import ( "os" "strconv" "testing" + "time" "github.com/harness/gitness/app/store" "github.com/harness/gitness/app/store/cache" @@ -80,7 +81,9 @@ func setupStores(t *testing.T, db *sqlx.DB) ( spacePathTransformation := store.ToLowerSpacePathTransformation spacePathStore := database.NewSpacePathStore(db, store.ToLowerSpacePathTransformation) - spacePathCache := cache.New(spacePathStore, spacePathTransformation) + + evictor := cache.NewEvictor[*types.SpaceCore]("namespace", "space-topic", nil) + spacePathCache := cache.New(context.Background(), spacePathStore, spacePathTransformation, evictor, time.Minute) spaceStore := database.NewSpaceStore(db, spacePathCache, spacePathStore) repoStore := database.NewRepoStore(db, spacePathCache, spacePathStore, spaceStore) diff --git a/cache/ttl_cache.go b/cache/ttl_cache.go index f2eba1f20..b338249b6 100644 --- a/cache/ttl_cache.go +++ b/cache/ttl_cache.go @@ -123,6 +123,12 @@ func (c *TTLCache[K, V]) Evict(_ context.Context, key K) { c.mx.Unlock() } +func (c *TTLCache[K, V]) EvictAll(_ context.Context) { + c.mx.Lock() + clear(c.cache) + c.mx.Unlock() +} + func (c *TTLCache[K, V]) fetch(key K, now time.Time) (V, bool) { c.mx.RLock() defer c.mx.RUnlock() diff --git a/cmd/gitness/wire.go b/cmd/gitness/wire.go index 3f279b6ad..8efb0d41a 100644 --- a/cmd/gitness/wire.go +++ b/cmd/gitness/wire.go @@ -147,7 +147,8 @@ func initSystem(ctx context.Context, config *types.Config) (*cliserver.System, e notification.WireSet, blob.WireSet, dbtx.WireSet, - cache.WireSet, + cache.WireSetSpace, + cache.WireSetRepo, refcache.WireSet, router.WireSet, pullreqservice.WireSet, diff --git a/cmd/gitness/wire_gen.go b/cmd/gitness/wire_gen.go index ebaafde8c..f25517934 100644 --- a/cmd/gitness/wire_gen.go +++ b/cmd/gitness/wire_gen.go @@ -149,26 +149,27 @@ func initSystem(ctx context.Context, config *types.Config) (*server.System, erro principalUID := check.ProvidePrincipalUIDCheck() spacePathTransformation := store.ProvidePathTransformation() spacePathStore := database.ProvideSpacePathStore(db, spacePathTransformation) - spacePathCache := cache.ProvidePathCache(spacePathStore, spacePathTransformation) - spaceStore := database.ProvideSpaceStore(db, spacePathCache, spacePathStore) - spaceIDCache := refcache.ProvideSpaceIDCache(spaceStore) - spaceRefCache := refcache.ProvideSpaceRefCache(spacePathStore, spacePathTransformation) pubsubConfig := server.ProvidePubsubConfig(config) universalClient, err := server.ProvideRedis(config) if err != nil { return nil, err } pubSub := pubsub.ProvidePubSub(pubsubConfig, universalClient) - spaceFinder := refcache.ProvideSpaceFinder(spaceIDCache, spaceRefCache, pubSub) + evictor := cache.ProvideEvictorSpaceCore(pubSub) + spacePathCache := cache.ProvideSpacePathCache(ctx, spacePathStore, evictor, spacePathTransformation) + spaceStore := database.ProvideSpaceStore(db, spacePathCache, spacePathStore) + spaceIDCache := cache.ProvideSpaceIDCache(ctx, spaceStore, evictor) + spaceFinder := refcache.ProvideSpaceFinder(spaceIDCache, spacePathCache, evictor) principalInfoView := database.ProvidePrincipalInfoView(db) principalInfoCache := cache.ProvidePrincipalInfoCache(principalInfoView) membershipStore := database.ProvideMembershipStore(db, principalInfoCache, spacePathStore, spaceStore) permissionCache := authz.ProvidePermissionCache(spaceFinder, membershipStore) publicAccessStore := database.ProvidePublicAccessStore(db) repoStore := database.ProvideRepoStore(db, spacePathCache, spacePathStore, spaceStore) - repoIDCache := refcache.ProvideRepoIDCache(repoStore) - repoRefCache := refcache.ProvideRepoRefCache(repoStore) - repoFinder := refcache.ProvideRepoFinder(repoStore, spaceRefCache, repoIDCache, repoRefCache, pubSub) + cacheEvictor := cache.ProvideEvictorRepositoryCore(pubSub) + repoIDCache := cache.ProvideRepoIDCache(ctx, repoStore, evictor, cacheEvictor) + repoRefCache := cache.ProvideRepoRefCache(ctx, repoStore, evictor, cacheEvictor) + repoFinder := refcache.ProvideRepoFinder(repoStore, spacePathCache, repoIDCache, repoRefCache, cacheEvictor) publicaccessService := publicaccess.ProvidePublicAccess(config, publicAccessStore, spaceFinder, repoFinder) authorizer := authz.ProvideAuthorizer(permissionCache, spaceFinder, publicaccessService) principalUIDTransformation := store.ProvidePrincipalUIDTransformation() diff --git a/types/repo.go b/types/repo.go index 32d1248cd..886aa3243 100644 --- a/types/repo.go +++ b/types/repo.go @@ -120,15 +120,11 @@ type RepoFilter struct { Recursive bool } -// RepositoryGitInfo holds git info for a repository. -type RepositoryGitInfo struct { - ID int64 - ParentID int64 - GitUID string +type RepoCacheKey struct { + SpaceID int64 + RepoIdentifier string } -func (rgi *RepositoryGitInfo) GetGitUID() string { return rgi.GitUID } - type RepositoryPullReqSummary struct { OpenCount int `json:"open_count"` ClosedCount int `json:"closed_count"`