diff options
author | Jacob Vosmaer <jacob@gitlab.com> | 2019-05-02 11:54:18 +0300 |
---|---|---|
committer | Jacob Vosmaer <jacob@gitlab.com> | 2019-05-02 11:54:18 +0300 |
commit | 6e1bd0c79267a70187f395d659c44ba005062f4a (patch) | |
tree | 2ac5d5595d60bd0a1b6e5f7ba0fe6c81fd84b7c1 | |
parent | be3c1e0908742d6c1150370e4c236446381d0683 (diff) | |
parent | 410326d5eb9c95ab263413005e502c9601c7760d (diff) |
Merge branch 'jc-fix-catfile-batch' into 'master'
Fix CacheItem pointer in cache
See merge request gitlab-org/gitaly!1234
-rw-r--r-- | changelogs/unreleased/jc-fix-catfile-batch.yml | 5 | ||||
-rw-r--r-- | internal/git/catfile/batch_cache.go | 6 | ||||
-rw-r--r-- | internal/git/catfile/batch_cache_test.go | 24 | ||||
-rw-r--r-- | internal/service/commit/find_commit_test.go | 51 |
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) + } +} |