Skip to content

Commit

Permalink
Change cache to avoid memory use
Browse files Browse the repository at this point in the history
Orignally, the cache was intended to be long lived to handle incoming webhooks
at any time. Currently, we are just polling, and just need the cache to handle
a single "EnforceAll" run, where we hit the same paths multiple times in that
run. Therefore, change the cache to be per-installation, and free it after each
"EnforceAll".

Signed-off-by: Jeff Mendoza <[email protected]>
  • Loading branch information
jeffmendoza committed Mar 7, 2024
1 parent 24b20ac commit 9c5f410
Show file tree
Hide file tree
Showing 3 changed files with 8 additions and 10 deletions.
2 changes: 1 addition & 1 deletion pkg/enforce/enforce.go
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,7 @@ func EnforceAll(ctx context.Context, ghc ghclients.GhClientsInterface, specificP
}
enforceAllResults[policyName]["totalFailed"] += results["totalFailed"]
}
ghc.Free(iid)
mu.Unlock()

if err != nil {
Expand All @@ -170,7 +171,6 @@ func EnforceAll(ctx context.Context, ghc ghclients.GhClientsInterface, specificP
if err := g.Wait(); err != nil {
return enforceAllResults, err
}
ghc.LogCacheSize()
log.Info().
Str("area", "bot").
Int("count", repoCount).
Expand Down
2 changes: 1 addition & 1 deletion pkg/enforce/enforce_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ func (m MockGhClients) Get(i int64) (*github.Client, error) {
return github.NewClient(&http.Client{}), nil
}

func (m MockGhClients) LogCacheSize() {}
func (m MockGhClients) Free(i int64) {}

func TestRunPolicies(t *testing.T) {
policiesGetPolicies = func() []policydef.Policy {
Expand Down
14 changes: 6 additions & 8 deletions pkg/ghclients/ghclients.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,15 +49,14 @@ func init() {

type GhClientsInterface interface {
Get(i int64) (*github.Client, error)
LogCacheSize()
Free(i int64)
}

// GHClients stores clients per-installation for re-use throughout a process.
type GHClients struct {
clients map[int64]*github.Client
tr http.RoundTripper
key []byte
cache *memoryCache
}

// NewGHClients returns a new GHClients. The provided RoundTripper will be
Expand All @@ -71,10 +70,13 @@ func NewGHClients(ctx context.Context, t http.RoundTripper) (*GHClients, error)
clients: make(map[int64]*github.Client),
tr: t,
key: key,
cache: newMemoryCache(),
}, nil
}

func (g *GHClients) Free(i int64) {
delete(g.clients, i)
}

// Get gets the client for installation id i, If i is 0 it gets the client for
// the app-level api. If a stored client is not available, it creates a new
// client with auth and caching built in.
Expand All @@ -85,7 +87,7 @@ func (g *GHClients) Get(i int64) (*github.Client, error) {

ctr := &httpcache.Transport{
Transport: g.tr,
Cache: g.cache,
Cache: newMemoryCache(),
MarkCachedResponses: true,
}

Expand All @@ -103,10 +105,6 @@ func (g *GHClients) Get(i int64) (*github.Client, error) {
return g.clients[i], nil
}

func (g *GHClients) LogCacheSize() {
g.cache.LogCacheSize()
}

func getKeyFromSecretReal(ctx context.Context, keySecretVal string) ([]byte, error) {
v, err := runtimevar.OpenVariable(ctx, keySecretVal)
if err != nil {
Expand Down

0 comments on commit 9c5f410

Please sign in to comment.