Limit Go-Git repo cache size and max in-memory file size (kptdev#1077)

* Limit repo cache size and max file size in go-git

Signed-off-by: Rendre Greyling <rendre.greyling@nokia.com>

* Add options to repo controller

Signed-off-by: Rendre Greyling <rendre.greyling@nokia.com>

* Fix lint issue

Signed-off-by: Rendre Greyling <rendre.greyling@nokia.com>

---------

Signed-off-by: Rendre Greyling <rendre.greyling@nokia.com>

Author: rendre-greyling

controllers/repositories/pkg/controllers/repository/cache.go

@@ -153,6 +153,8 @@ func (r *RepositoryReconciler) buildCacheOptions(
 			CaBundleResolver:           caBundleResolver,
 			UserInfoProvider:           userInfoProvider,
 			RepoOperationRetryAttempts: r.RepoOperationRetryAttempts,
+			GoGitRepoCacheSize:         r.GoGitRepoCacheSize,
+			GoGitCacheMaxFileSize:      r.GoGitCacheMaxFileSize,
 		},
 		RepoPRChangeNotifier: cachetypes.NewNoOpRepoPRChangeNotifier(),
 		DbPushDraftsToGit:    r.PushDraftsToGit,

controllers/repositories/pkg/controllers/repository/config.go

@@ -29,6 +29,8 @@ const (
 	defaultSyncStaleTimeout           = 20 * time.Minute
 	defaultRepoOperationRetryAttempts = 3
 	defaultCacheDirectory             = "/cache"
+	defaultGoGitRepoCacheSize         = 8              // MiB
+	defaultGoGitCacheMaxFileSize      = 1 * 1024 * 512 // bytes (512 KiB)
 )
 
 // InitDefaults initializes default values for standalone controller
@@ -41,6 +43,8 @@ func (r *RepositoryReconciler) InitDefaults() {
 	r.RepoOperationRetryAttempts = defaultRepoOperationRetryAttempts
 	r.cacheType = string(cachetypes.DBCacheType)
 	r.cacheDirectory = defaultCacheDirectory
+	r.GoGitRepoCacheSize = defaultGoGitRepoCacheSize
+	r.GoGitCacheMaxFileSize = defaultGoGitCacheMaxFileSize
 	r.validateConfig()
 }
 
@@ -57,6 +61,8 @@ func (r *RepositoryReconciler) BindFlags(prefix string, flags *flag.FlagSet) {
 	flags.BoolVar(&r.useUserDefinedCaBundle, prefix+"use-user-defined-ca-bundle", false, "Enable custom CA bundle support from secrets")
 	flags.BoolVar(&r.CreateV1Alpha2Rpkg, prefix+"create-v1alpha2-rpkg", false, "Create v1alpha2 PackageRevision resources during repository sync")
 	flags.BoolVar(&r.PushDraftsToGit, prefix+"push-drafts-to-git", false, "Push draft and proposed branches to git when using DB cache")
+	flags.IntVar(&r.GoGitRepoCacheSize, prefix+"gogit-repo-cache-size", defaultGoGitRepoCacheSize, "Size of the in-memory cache for git repositories when using gogit (in MiB)")
+	flags.Int64Var(&r.GoGitCacheMaxFileSize, prefix+"gogit-cache-max-file-size", defaultGoGitCacheMaxFileSize, "Maximum file size (in bytes) that will be read into the in-memory cache for git repositories when using gogit; files larger than this will be streamed from disk")
 }
 
 // validateConfig ensures configuration values are valid
@@ -95,7 +101,9 @@ func (r *RepositoryReconciler) LogConfig(log interface {
 		"cacheType", r.cacheType,
 		"cacheDirectory", r.cacheDirectory,
 		"createV1Alpha2Rpkg", r.CreateV1Alpha2Rpkg,
-		"pushDraftsToGit", r.PushDraftsToGit)
+		"pushDraftsToGit", r.PushDraftsToGit,
+		"goGitRepoCacheSize", r.GoGitRepoCacheSize,
+		"goGitCacheMaxFileSize", r.GoGitCacheMaxFileSize)
 
 	if r.HealthCheckFrequency < defaultHealthCheckFrequency {
 		log.Info("Health check frequency is lower than recommended default",

controllers/repositories/pkg/controllers/repository/repository_controller.go

@@ -60,6 +60,10 @@ type RepositoryReconciler struct {
 	CreateV1Alpha2Rpkg bool // Create v1alpha2 PackageRevision resources during repo sync
 	PushDraftsToGit    bool // Push draft/proposed branches to git (DB cache only)
 
+	// GoGit cache configuration
+	GoGitRepoCacheSize    int   // In-memory cache size for git repositories (MiB)
+	GoGitCacheMaxFileSize int64 // Max file size (bytes) to read into the in-memory git cache
+
 	// Configuration (set via flags or defaults)
 	cacheType              string // Cache type (DB or CR)
 	cacheDirectory         string // Directory for git repository cache

pkg/cmd/server/start.go

@@ -68,6 +68,9 @@ type PorchServerOptions struct {
 	DbMaxConnLifetime    time.Duration
 	DbPushDrafsToGit     bool
 
+	GoGitRepoCacheSize    int
+	GoGitCacheMaxFileSize int64
+
 	DefaultImagePrefix       string
 	FunctionRunnerAddress    string
 	LocalStandaloneDebugging bool // Enables local standalone running/debugging of the apiserver.
@@ -331,6 +334,8 @@ func (o *PorchServerOptions) Config() (*apiserver.Config, error) {
 				ExternalRepoOptions: externalrepotypes.ExternalRepoOptions{
 					LocalDirectory:         o.CacheDirectory,
 					UseUserDefinedCaBundle: o.UseUserDefinedCaBundle,
+					GoGitRepoCacheSize:     o.GoGitRepoCacheSize,
+					GoGitCacheMaxFileSize:  o.GoGitCacheMaxFileSize,
 				},
 				RepoOperationRetryAttempts: o.RepoOperationRetryAttempts,
 				CacheType:                  cachetypes.CacheType(o.CacheType),
@@ -395,6 +400,10 @@ func (o *PorchServerOptions) AddFlags(fs *pflag.FlagSet) {
 	fs.StringVar(&o.DbCacheDataSource, "db-cache-data-source", "", "Address of the database, for example \"postgresql://user:pass@hostname:port/database\"")
 	fs.BoolVar(&o.DbPushDrafsToGit, "db-push-drafts-to-git", false, "If true, Porch will push draft package revisions to git when using the DB cache")
 
+	//GoGit configuration
+	fs.IntVar(&o.GoGitRepoCacheSize, "gogit-repo-cache-size", 8, "Size of the in-memory cache for git repositories when using gogit (in MiB)")
+	fs.Int64Var(&o.GoGitCacheMaxFileSize, "gogit-cache-max-file-size", 1*1024*512, "Maximum file size (in bytes) that will be read into the in-memory cache for git repositories when using gogit; files larger than this will be streamed from disk to avoid memory pressure")
+
 	// Function runner configuration
 	fs.StringVar(&o.DefaultImagePrefix, "default-image-prefix", runneroptions.GHCRImagePrefix, "Default prefix for unqualified function names")
 	fs.StringVar(&o.FunctionRunnerAddress, "function-runner", "", "Address of the function runner gRPC service.")

pkg/externalrepo/git/cachedir_pool.go

@@ -41,7 +41,7 @@ var globalDirectoryPool = &directoryPool{
 }
 
 // getOrCreateSharedRepository safely initializes or reuses a cached git directory
-func (p *directoryPool) getOrCreateSharedRepository(dir, reponame string) (*sharedDirectory, error) {
+func (p *directoryPool) getOrCreateSharedRepository(dir, reponame string, opts GitRepositoryOptions) (*sharedDirectory, error) {
 	// Fast path: check if directory already exists
 	if sharedDir, exists := p.directories.Load(dir); exists {
 		p.mutex.Lock()
@@ -72,7 +72,7 @@ func (p *directoryPool) getOrCreateSharedRepository(dir, reponame string) (*shar
 		if !os.IsNotExist(err) {
 			return nil, err
 		}
-		repo, err = initEmptyRepository(dir)
+		repo, err = initEmptyRepository(dir, opts)
 		if err != nil {
 			if removeErr := os.RemoveAll(dir); removeErr != nil {
 				klog.Errorf("Failed to remove partially created directory %s: %v", dir, removeErr)
@@ -82,7 +82,7 @@ func (p *directoryPool) getOrCreateSharedRepository(dir, reponame string) (*shar
 	} else if !fi.IsDir() {
 		return nil, fmt.Errorf("cache location %q is not a directory", dir)
 	} else {
-		repo, err = openRepository(dir)
+		repo, err = openRepository(dir, opts)
 		if err != nil {
 			if removeErr := os.RemoveAll(dir); removeErr != nil {
 				klog.Errorf("Failed to open repository %s: %v (also failed to remove corrupted directory: %v)", dir, err, removeErr)

pkg/externalrepo/git/cachedir_pool_test.go

@@ -25,6 +25,8 @@ import (
 	"github.com/stretchr/testify/require"
 )
 
+var opts = GitRepositoryOptions{}
+
 func TestDirectoryPool_GetOrCreateSharedRepository(t *testing.T) {
 	pool := &directoryPool{
 		directories: sync.Map{},
@@ -34,13 +36,13 @@ func TestDirectoryPool_GetOrCreateSharedRepository(t *testing.T) {
 	repoDir := filepath.Join(tempDir, "test-repo")
 
 	// First call should create new shared directory
-	shared1, err := pool.getOrCreateSharedRepository(repoDir, "test-repo")
+	shared1, err := pool.getOrCreateSharedRepository(repoDir, "test-repo", opts)
 	require.NoError(t, err)
 	require.NotNil(t, shared1)
 	assert.Equal(t, 1, shared1.refCount)
 
 	// Second call should reuse existing directory
-	shared2, err := pool.getOrCreateSharedRepository(repoDir, "test-repo")
+	shared2, err := pool.getOrCreateSharedRepository(repoDir, "test-repo", opts)
 	require.NoError(t, err)
 	assert.Same(t, shared1, shared2)
 	assert.Equal(t, 2, shared2.refCount)
@@ -55,11 +57,11 @@ func TestDirectoryPool_ReleaseSharedRepository(t *testing.T) {
 	repoDir := filepath.Join(tempDir, "test-repo")
 
 	// Create shared repository
-	shared, err := pool.getOrCreateSharedRepository(repoDir, "test-repo")
+	shared, err := pool.getOrCreateSharedRepository(repoDir, "test-repo", opts)
 	require.NoError(t, err)
 
 	// Add another reference
-	_, err = pool.getOrCreateSharedRepository(repoDir, "test-repo")
+	_, err = pool.getOrCreateSharedRepository(repoDir, "test-repo", opts)
 	require.NoError(t, err)
 
 	// First release should decrement refCount
@@ -76,7 +78,7 @@ func TestDirectoryPool_ReleaseSharedRepository(t *testing.T) {
 
 func TestSharedDirectory_WithLock(t *testing.T) {
 	tempDir := t.TempDir()
-	repo, err := initEmptyRepository(tempDir)
+	repo, err := initEmptyRepository(tempDir, opts)
 	require.NoError(t, err)
 
 	shared := &sharedDirectory{
@@ -97,7 +99,7 @@ func TestSharedDirectory_WithLock(t *testing.T) {
 
 func TestSharedDirectory_WithRLock(t *testing.T) {
 	tempDir := t.TempDir()
-	repo, err := initEmptyRepository(tempDir)
+	repo, err := initEmptyRepository(tempDir, opts)
 	require.NoError(t, err)
 
 	shared := &sharedDirectory{
@@ -132,7 +134,7 @@ func TestDirectoryPool_ConcurrentAccess(t *testing.T) {
 		wg.Add(1)
 		go func(id int) {
 			defer wg.Done()
-			_, err := pool.getOrCreateSharedRepository(repoDir, "concurrent-repo")
+			_, err := pool.getOrCreateSharedRepository(repoDir, "concurrent-repo", opts)
 			assert.NoError(t, err)
 		}(i)
 	}
@@ -174,7 +176,7 @@ func TestDirectoryPool_InitEmptyRepositoryFailure(t *testing.T) {
 	repoDir := filepath.Join(filePath, "subdir") // This will fail because parent is a file
 
 	// Should fail to create repository
-	_, err = pool.getOrCreateSharedRepository(repoDir, "test-repo")
+	_, err = pool.getOrCreateSharedRepository(repoDir, "test-repo", opts)
 	assert.Error(t, err)
 
 	// Should not be in pool
@@ -183,7 +185,7 @@ func TestDirectoryPool_InitEmptyRepositoryFailure(t *testing.T) {
 
 	// Retry should work if we fix the issue
 	os.Remove(filePath)
-	_, err = pool.getOrCreateSharedRepository(repoDir, "test-repo")
+	_, err = pool.getOrCreateSharedRepository(repoDir, "test-repo", opts)
 	assert.NoError(t, err)
 }
 
@@ -203,7 +205,7 @@ func TestDirectoryPool_OpenRepositoryFailure(t *testing.T) {
 	require.NoError(t, err)
 
 	// Should fail to open corrupted repository
-	_, err = pool.getOrCreateSharedRepository(repoDir, "test-repo")
+	_, err = pool.getOrCreateSharedRepository(repoDir, "test-repo", opts)
 	assert.Error(t, err)
 	assert.Contains(t, err.Error(), "corrupted cache was removed")
 
@@ -216,7 +218,7 @@ func TestDirectoryPool_OpenRepositoryFailure(t *testing.T) {
 	assert.True(t, os.IsNotExist(err))
 
 	// Retry should work now that corrupted dir is removed
-	_, err = pool.getOrCreateSharedRepository(repoDir, "test-repo")
+	_, err = pool.getOrCreateSharedRepository(repoDir, "test-repo", opts)
 	assert.NoError(t, err)
 }
 
@@ -242,7 +244,7 @@ func TestDirectoryPool_OpenRepositoryCleanupFailure(t *testing.T) {
 	defer os.Chmod(repoDir, 0755)
 
 	// Should fail to open and also fail to cleanup
-	_, err = pool.getOrCreateSharedRepository(repoDir, "test-repo")
+	_, err = pool.getOrCreateSharedRepository(repoDir, "test-repo", opts)
 	assert.Error(t, err)
 	// When cleanup fails, error message should mention checking local cache
 	assert.Contains(t, err.Error(), "check the local git cache")
@@ -269,7 +271,7 @@ func TestDirectoryPool_PathIsFile(t *testing.T) {
 	require.NoError(t, err)
 
 	// Should fail because path is a file, not a directory
-	_, err = pool.getOrCreateSharedRepository(filePath, "test-repo")
+	_, err = pool.getOrCreateSharedRepository(filePath, "test-repo", opts)
 	assert.Error(t, err)
 	assert.Contains(t, err.Error(), "not a directory")
 

pkg/externalrepo/git/git.go

@@ -159,7 +159,7 @@ func OpenRepository(ctx context.Context, name, namespace string, spec *configapi
 	}
 
 	// Get or create shared repository - let SharedDirectory handle cleanup
-	sharedDir, err := globalDirectoryPool.getOrCreateSharedRepository(dir, name)
+	sharedDir, err := globalDirectoryPool.getOrCreateSharedRepository(dir, name, opts)
 	if err != nil {
 		return nil, pkgerrors.Wrapf(err, "error cloning git repository %+v", spec.Repo)
 	}

pkg/externalrepo/git/gogit.go

@@ -27,7 +27,7 @@ import (
 
 // This file contains helpers for interacting with gogit.
 
-func initEmptyRepository(path string) (*git.Repository, error) {
+func initEmptyRepository(path string, opts GitRepositoryOptions) (*git.Repository, error) {
 	isBare := true // Porch only uses bare repositories
 	repo, err := git.PlainInit(path, isBare)
 	if err != nil {
@@ -36,7 +36,9 @@ func initEmptyRepository(path string) (*git.Repository, error) {
 	if err := initializeDefaultBranches(repo); err != nil {
 		return nil, pkgerrors.Wrapf(err, "gogit: default branch initialize failed on repo for path %q", path)
 	}
-	return repo, nil
+	// Re-open with tuned storage settings to avoid the default 96MiB in-memory LRU cache
+	// that PlainInit creates, since this repo will be long-lived in the directory pool.
+	return openRepository(path, opts)
 }
 
 func initializeDefaultBranches(repo *git.Repository) error {
@@ -52,11 +54,28 @@ func initializeDefaultBranches(repo *git.Repository) error {
 	return nil
 }
 
-func openRepository(path string) (*git.Repository, error) {
+func openRepository(path string, opts GitRepositoryOptions) (*git.Repository, error) {
 	dot := osfs.New(path)
-	storage := filesystem.NewStorageWithOptions(dot, cache.NewObjectLRUDefault(), filesystem.Options{
-		ExclusiveAccess: true,
-	})
+
+	const (
+		defaultRepoCacheSizeMiB = 8
+		defaultMaxFileSize      = 512 * 1024
+	)
+	cacheSizeMiB := opts.GoGitRepoCacheSize
+	if cacheSizeMiB <= 0 {
+		cacheSizeMiB = defaultRepoCacheSizeMiB
+	}
+	maxFileSize := opts.GoGitCacheMaxFileSize
+	if maxFileSize <= 0 {
+		maxFileSize = defaultMaxFileSize
+	}
+	objectCache := cache.NewObjectLRU(cache.FileSize(cacheSizeMiB) * cache.MiByte)
+
+	fileOpts := filesystem.Options{
+		ExclusiveAccess:      true,
+		LargeObjectThreshold: maxFileSize,
+	}
+	storage := filesystem.NewStorageWithOptions(dot, objectCache, fileOpts)
 	return git.Open(storage, dot)
 }
 

pkg/externalrepo/types/externalrepotypes.go

@@ -33,4 +33,6 @@ type ExternalRepoOptions struct {
 	CaBundleResolver           repository.CredentialResolver
 	UserInfoProvider           repository.UserInfoProvider
 	RepoOperationRetryAttempts int
+	GoGitRepoCacheSize         int
+	GoGitCacheMaxFileSize      int64
 }