Merge branch 'ps-dist-reads-to-primary' into 'master'

Distribute reads between all shards, including primaries

Closes #2834

See merge request gitlab-org/gitaly!2267

3 files changed, 12 insertions, 8 deletions

diff --git a/changelogs/unreleased/ps-dist-reads-to-primary.yml b/changelogs/unreleased/ps-dist-reads-to-primary.yml
new file mode 100644
index 000000000..82965ccd4
--- /dev/null
+++ b/changelogs/unreleased/ps-dist-reads-to-primary.yml
@@ -0,0 +1,5 @@
+---
+title: Distribute reads between all shards, including primaries
+merge_request: 2267
+author:
+type: added
diff --git a/internal/praefect/nodes/manager.go b/internal/praefect/nodes/manager.go
index a08a732d3..0d9fc6e0a 100644
--- a/internal/praefect/nodes/manager.go
+++ b/internal/praefect/nodes/manager.go
@@ -233,7 +233,9 @@ func (n *Mgr) GetSyncedNode(ctx context.Context, virtualStorageName, repoPath st
 		logger.WithError(err).Warn("get up to date secondaries")
 	}
 
-	var storages []Node
+	storages := make([]Node, 0, len(upToDateStorages)+1) // +1 is for the primary node
+	storages = append(storages, shard.Primary)
+
 	for _, upToDateStorage := range upToDateStorages {
 		node, err := shard.GetNode(upToDateStorage)
 		if err != nil {
@@ -241,18 +243,14 @@ func (n *Mgr) GetSyncedNode(ctx context.Context, virtualStorageName, repoPath st
 			logger.WithError(err).Warn("storage returned as up-to-date")
 		}
 
-		if !node.IsHealthy() || node == shard.Primary {
+		if !node.IsHealthy() {
 			continue
 		}
 
 		storages = append(storages, node)
 	}
 
-	if len(storages) == 0 {
-		return shard.Primary, nil
-	}
-
-	return storages[rand.Intn(len(storages))], nil // randomly pick up one of the synced storages
+	return storages[rand.Intn(len(storages))], nil
 }
 
 func newConnectionStatus(node config.Node, cc *grpc.ClientConn, l logrus.FieldLogger, latencyHist prommetrics.HistogramVec) *nodeStatus {
diff --git a/internal/praefect/nodes/manager_test.go b/internal/praefect/nodes/manager_test.go
index 199cebb10..8fbd1c56f 100644
--- a/internal/praefect/nodes/manager_test.go
+++ b/internal/praefect/nodes/manager_test.go
@@ -385,6 +385,7 @@ func TestMgr_GetSyncedNode(t *testing.T) {
 	}
 
 	vs0Primary := "unix://" + sockets[0]
+	vs1Primary := "unix://" + sockets[2]
 	vs1Secondary := "unix://" + sockets[3]
 
 	virtualStorages := []*config.VirtualStorage{
@@ -586,7 +587,7 @@ func TestMgr_GetSyncedNode(t *testing.T) {
 
 		node, err := nm.GetSyncedNode(ctx, vs1Event.Job.VirtualStorage, vs1Event.Job.RelativePath)
 		require.NoError(t, err)
-		require.Equal(t, vs1Secondary, node.GetAddress())
+		require.Contains(t, []string{vs1Primary, vs1Secondary}, node.GetAddress(), "should be one of the secondaries or the primary")
 	}))
 }