Merge pull request '[ENHANCEMENT] Improve caching of contributor stats' (#4367) from gusted/contributors-stats-cache into forgejo

Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/4367
Reviewed-by: Earl Warren <earl-warren@noreply.codeberg.org>

release-notes/9.0.0/4367.md

@@ -0,0 +1 @@
+The caching of contributor stats was improved (the data used by `/<user>/<repo>/activity/recent-commits`) to use the configured cache TTL from the config (`[cache].ITEM_TTL`) instead of a hardcoded TTL of ten minutes. The computation of this operation is computationally heavy and makes a lot of requests to the database and Git on repositories with a lot of commits. It should be cached for longer than what was previously hardcoded, ten minutes.

services/repository/contributors_graph.go

@@ -22,15 +22,13 @@ import (
 	"code.gitea.io/gitea/modules/graceful"
 	"code.gitea.io/gitea/modules/json"
 	"code.gitea.io/gitea/modules/log"
+	"code.gitea.io/gitea/modules/setting"
 	api "code.gitea.io/gitea/modules/structs"
 
 	"gitea.com/go-chi/cache"
 )
 
-const (
+const contributorStatsCacheKey = "GetContributorStats/%s/%s"
-	contributorStatsCacheKey           = "GetContributorStats/%s/%s"
-	contributorStatsCacheTimeout int64 = 60 * 10
-)
 
 var (
 	ErrAwaitGeneration  = errors.New("generation took longer than ")
@@ -211,8 +209,7 @@ func generateContributorStats(genDone chan struct{}, cache cache.Cache, cacheKey
 
 	gitRepo, closer, err := gitrepo.RepositoryFromContextOrOpen(ctx, repo)
 	if err != nil {
-		err := fmt.Errorf("OpenRepository: %w", err)
+		log.Error("OpenRepository[repo=%q]: %v", repo.FullName(), err)
-		_ = cache.Put(cacheKey, err, contributorStatsCacheTimeout)
 		return
 	}
 	defer closer.Close()
@@ -222,13 +219,11 @@ func generateContributorStats(genDone chan struct{}, cache cache.Cache, cacheKey
 	}
 	extendedCommitStats, err := getExtendedCommitStats(gitRepo, revision)
 	if err != nil {
-		err := fmt.Errorf("ExtendedCommitStats: %w", err)
+		log.Error("getExtendedCommitStats[repo=%q revision=%q]: %v", repo.FullName(), revision, err)
-		_ = cache.Put(cacheKey, err, contributorStatsCacheTimeout)
 		return
 	}
 	if len(extendedCommitStats) == 0 {
-		err := fmt.Errorf("no commit stats returned for revision '%s'", revision)
+		log.Error("No commit stats were returned [repo=%q revision=%q]", repo.FullName(), revision)
-		_ = cache.Put(cacheKey, err, contributorStatsCacheTimeout)
 		return
 	}
 
@@ -312,14 +307,13 @@ func generateContributorStats(genDone chan struct{}, cache cache.Cache, cacheKey
 
 	data, err := json.Marshal(contributorsCommitStats)
 	if err != nil {
-		err := fmt.Errorf("couldn't marshal the data: %w", err)
+		log.Error("json.Marshal[repo=%q revision=%q]: %v", repo.FullName(), revision, err)
-		_ = cache.Put(cacheKey, err, contributorStatsCacheTimeout)
 		return
 	}
 
 	// Store the data as an string, to make it uniform what data type is returned
 	// from caches.
-	_ = cache.Put(cacheKey, string(data), contributorStatsCacheTimeout)
+	_ = cache.Put(cacheKey, string(data), setting.CacheService.TTLSeconds())
 	generateLock.Delete(cacheKey)
 	if genDone != nil {
 		genDone <- struct{}{}

services/repository/contributors_graph_test.go

@@ -6,12 +6,14 @@ package repository
 import (
 	"slices"
 	"testing"
+	"time"
 
 	"code.gitea.io/gitea/models/db"
 	repo_model "code.gitea.io/gitea/models/repo"
 	"code.gitea.io/gitea/models/unittest"
-	"code.gitea.io/gitea/modules/git"
 	"code.gitea.io/gitea/modules/json"
+	"code.gitea.io/gitea/modules/log"
+	"code.gitea.io/gitea/modules/test"
 
 	"gitea.com/go-chi/cache"
 	"github.com/stretchr/testify/assert"
@@ -27,10 +29,14 @@ func TestRepository_ContributorsGraph(t *testing.T) {
 	})
 	assert.NoError(t, err)
 
+	lc, cleanup := test.NewLogChecker(log.DEFAULT, log.INFO)
+	lc.StopMark(`getExtendedCommitStats[repo="user2/repo2" revision="404ref"]: object does not exist [id: 404ref, rel_path: ]`)
+	defer cleanup()
+
 	generateContributorStats(nil, mockCache, "key", repo, "404ref")
-	err, isErr := mockCache.Get("key").(error)
+	assert.False(t, mockCache.IsExist("key"))
-	assert.True(t, isErr)
+	_, stopped := lc.Check(100 * time.Millisecond)
-	assert.ErrorAs(t, err, &git.ErrNotExist{})
+	assert.True(t, stopped)
 
 	generateContributorStats(nil, mockCache, "key2", repo, "master")
 	dataString, isData := mockCache.Get("key2").(string)