Fix CacheItem pointer in cache

4 files changed, 82 insertions, 4 deletions

diff --git a/changelogs/unreleased/jc-fix-catfile-batch.yml b/changelogs/unreleased/jc-fix-catfile-batch.yml
new file mode 100644
index 000000000..a9c51c055
--- /dev/null
+++ b/changelogs/unreleased/jc-fix-catfile-batch.yml
@@ -0,0 +1,5 @@
+---
+title: Fix CacheItem pointer in cache
+merge_request: 1234
+author:
+type: fixed
diff --git a/internal/git/catfile/batch_cache.go b/internal/git/catfile/batch_cache.go
index 8028cedd3..b0db3b289 100644
--- a/internal/git/catfile/batch_cache.go
+++ b/internal/git/catfile/batch_cache.go
@@ -64,7 +64,7 @@ type BatchCache struct {
 }
 
 func closeBatchAndStopTTL(key lru.Key, value interface{}) {
-	cacheItem := value.(CacheItem)
+	cacheItem := value.(*CacheItem)
 	close(cacheItem.stopTTL)
 
 	if !cacheItem.preserveBatchOnEvict {
@@ -97,7 +97,7 @@ func (bc *BatchCache) Get(key CacheKey) *Batch {
 		return nil
 	}
 
-	cacheItem := v.(CacheItem)
+	cacheItem := v.(*CacheItem)
 
 	// set preserveBatchOnEvict=true so that OnEvict doesn't close the batch
 	cacheItem.preserveBatchOnEvict = true
@@ -123,7 +123,7 @@ func (bc *BatchCache) Add(key CacheKey, b *Batch, ttl time.Duration) {
 
 	stopTTL := make(chan struct{})
 
-	bc.lru.Add(key, CacheItem{
+	bc.lru.Add(key, &CacheItem{
 		batch:   b,
 		stopTTL: stopTTL,
 	})
diff --git a/internal/git/catfile/batch_cache_test.go b/internal/git/catfile/batch_cache_test.go
index 2b6353ea4..0de0a9469 100644
--- a/internal/git/catfile/batch_cache_test.go
+++ b/internal/git/catfile/batch_cache_test.go
@@ -33,3 +33,27 @@ func TestBatchCacheTTL(t *testing.T) {
 
 	assert.Nil(t, batchCache.Get(cacheKey))
 }
+
+func TestBatchCache(t *testing.T) {
+	ctx, cancel := testhelper.Context()
+	defer cancel()
+
+	testRepo, _, cleanupFn := testhelper.NewTestRepo(t)
+	defer cleanupFn()
+
+	batchCache := catfile.NewCache(10)
+
+	c, err := catfile.New(ctx, testRepo)
+	require.NoError(t, err)
+
+	sessionID := "abcdefg1231231"
+
+	ttl := 10 * time.Second
+
+	cacheKey := catfile.NewCacheKey(sessionID, testRepo)
+	batchCache.Add(cacheKey, c, ttl)
+	b := batchCache.Get(cacheKey)
+	defer b.Close()
+
+	assert.Equal(t, c, b)
+}
diff --git a/internal/service/commit/find_commit_test.go b/internal/service/commit/find_commit_test.go
index 73eba3d73..ab64427c7 100644
--- a/internal/service/commit/find_commit_test.go
+++ b/internal/service/commit/find_commit_test.go
@@ -331,7 +331,11 @@ func benchmarkFindCommit(withCache bool, b *testing.B) {
 	for i := 0; i < b.N; i++ {
 		revision := revisions[b.N%len(revisions)]
 		if withCache {
-			md := metadata.New(map[string]string{featureflag.HeaderKey(catfile.CacheFeatureFlagKey): "true", "gitaly-session-id": "abc123"})
+			md := metadata.New(map[string]string{
+				featureflag.HeaderKey(catfile.CacheFeatureFlagKey): "true",
+				"gitaly-session-id": "abc123",
+			})
+
 			ctx = metadata.NewOutgoingContext(ctx, md)
 		}
 		_, err := client.FindCommit(ctx, &gitalypb.FindCommitRequest{
@@ -341,3 +345,48 @@ func benchmarkFindCommit(withCache bool, b *testing.B) {
 		require.NoError(b, err)
 	}
 }
+
+func TestFindCommitWithCache(t *testing.T) {
+	ctx, cancel := testhelper.Context()
+	defer cancel()
+
+	server, serverSocketPath := startTestServices(t)
+	defer server.Stop()
+
+	client, conn := newCommitServiceClient(t, serverSocketPath)
+	defer conn.Close()
+
+	testRepo, _, cleanupFn := testhelper.NewTestRepo(t)
+	defer cleanupFn()
+
+	// get a list of revisions
+
+	logCmd, err := git.Command(ctx, testRepo, "log", "--format=format:%H")
+	require.NoError(t, err)
+
+	logScanner := bufio.NewScanner(logCmd)
+
+	var revisions []string
+	for logScanner.Scan() {
+		revisions = append(revisions, logScanner.Text())
+	}
+
+	require.NoError(t, logCmd.Wait())
+
+	defer catfile.ExpireAll()
+
+	for i := 0; i < 10; i++ {
+		revision := revisions[i%len(revisions)]
+		md := metadata.New(map[string]string{
+			featureflag.HeaderKey(catfile.CacheFeatureFlagKey): "true",
+			"gitaly-session-id": "abc123",
+		})
+
+		ctx = metadata.NewOutgoingContext(ctx, md)
+		_, err := client.FindCommit(ctx, &gitalypb.FindCommitRequest{
+			Repository: testRepo,
+			Revision:   []byte(revision),
+		})
+		require.NoError(t, err)
+	}
+}