Merge branch 'pks-catfile-object-reader-in-use' into 'master'

catfile: Fix ObjectReader not being dirty with in-use queue

Closes #5543

See merge request https://gitlab.com/gitlab-org/gitaly/-/merge_requests/6282

Merged-by: Quang-Minh Nguyen <qmnguyen@gitlab.com>
Approved-by: karthik nayak <knayak@gitlab.com>
Approved-by: Quang-Minh Nguyen <qmnguyen@gitlab.com>
Co-authored-by: Patrick Steinhardt <psteinhardt@gitlab.com>

2 files changed, 24 insertions, 1 deletions

diff --git a/internal/git/catfile/object_reader.go b/internal/git/catfile/object_reader.go
index d431b0576..cf467af58 100644
--- a/internal/git/catfile/object_reader.go
+++ b/internal/git/catfile/object_reader.go
@@ -112,6 +112,10 @@ func (o *objectReader) isClosed() bool {
 }
 
 func (o *objectReader) isDirty() bool {
+	if atomic.LoadInt32(&o.queueInUse) != 0 {
+		return true
+	}
+
 	return o.queue.isDirty()
 }
 
diff --git a/internal/git/catfile/object_reader_test.go b/internal/git/catfile/object_reader_test.go
index e171bff31..e98fc9831 100644
--- a/internal/git/catfile/object_reader_test.go
+++ b/internal/git/catfile/object_reader_test.go
@@ -258,6 +258,18 @@ func TestObjectReader_queue(t *testing.T) {
 	foobarBlob := gittest.WriteBlob(t, cfg, repoPath, []byte("foobar"))
 	barfooBlob := gittest.WriteBlob(t, cfg, repoPath, []byte("barfoo"))
 
+	t.Run("reader is dirty with acquired queue", func(t *testing.T) {
+		reader, err := newObjectReader(ctx, cmdExecutor, nil)
+		require.NoError(t, err)
+
+		queue, cleanup, err := reader.objectQueue(ctx, "trace")
+		require.NoError(t, err)
+		defer cleanup()
+
+		require.False(t, queue.isDirty())
+		require.True(t, reader.isDirty())
+	})
+
 	t.Run("read single object", func(t *testing.T) {
 		reader, err := newObjectReader(ctx, cmdExecutor, nil)
 		require.NoError(t, err)
@@ -541,11 +553,13 @@ func TestObjectReader_queue(t *testing.T) {
 		reader, err := newObjectReader(ctx, cmdExecutor, nil)
 		require.NoError(t, err)
 
+		require.False(t, reader.isDirty())
+
 		queue, cleanup, err := reader.objectQueue(ctx, "trace")
 		require.NoError(t, err)
 		defer cleanup()
 
-		require.False(t, reader.isDirty())
+		require.True(t, reader.isDirty())
 		require.False(t, queue.isDirty())
 
 		require.NoError(t, queue.RequestObject(ctx, foobarBlob.Revision()))
@@ -564,6 +578,11 @@ func TestObjectReader_queue(t *testing.T) {
 		_, err = io.ReadAll(object)
 		require.NoError(t, err)
 
+		require.True(t, reader.isDirty())
+		require.False(t, queue.isDirty())
+
+		cleanup()
+
 		require.False(t, reader.isDirty())
 		require.False(t, queue.isDirty())
 	})