fix(cluster): reduce lock contention on cluster initialization (#660)

* fix: move expensive function outside lock Signed-off-by: Michael Crenshaw <[email protected]> * add benchmark Signed-off-by: Michael Crenshaw <[email protected]> --------- Signed-off-by: Michael Crenshaw <[email protected]>
argoproj · Jan 24, 2025 · d78929e · d78929e
1 parent 54992bf
commit d78929e
Show file tree

Hide file tree

Showing 2 changed files with 44 additions and 1 deletion.
diff --git a/pkg/cache/cluster.go b/pkg/cache/cluster.go
@@ -939,8 +939,9 @@ func (c *clusterCache) sync() error {
 					if un, ok := obj.(*unstructured.Unstructured); !ok {
 						return fmt.Errorf("object %s/%s has an unexpected type", un.GroupVersionKind().String(), un.GetName())
 					} else {
+						newRes := c.newResource(un)
 						lock.Lock()
-						c.setNode(c.newResource(un))
+						c.setNode(newRes)
 						lock.Unlock()
 					}
 					return nil

diff --git a/pkg/cache/cluster_test.go b/pkg/cache/cluster_test.go
@@ -146,6 +146,48 @@ func getChildren(cluster *clusterCache, un *unstructured.Unstructured) []*Resour
 	return hierarchy[1:]
 }
 
+// Benchmark_sync is meant to simulate cluster initialization when populateResourceInfoHandler does nontrivial work.
+func Benchmark_sync(t *testing.B) {
+	var resources = []runtime.Object{}
+	for i := 0; i < 100; i++ {
+		resources = append(resources, &corev1.Pod{
+			ObjectMeta: metav1.ObjectMeta{
+				Name:      fmt.Sprintf("pod-%d", i),
+				Namespace: "default",
+			},
+		}, &appsv1.ReplicaSet{
+			ObjectMeta: metav1.ObjectMeta{
+				Name:      fmt.Sprintf("rs-%d", i),
+				Namespace: "default",
+			},
+		}, &appsv1.Deployment{
+			ObjectMeta: metav1.ObjectMeta{
+				Name:      fmt.Sprintf("deploy-%d", i),
+				Namespace: "default",
+			},
+		}, &appsv1.StatefulSet{
+			ObjectMeta: metav1.ObjectMeta{
+				Name:      fmt.Sprintf("sts-%d", i),
+				Namespace: "default",
+			},
+		})
+	}
+
+	c := newCluster(t, resources...)
+
+	c.populateResourceInfoHandler = func(un *unstructured.Unstructured, isRoot bool) (info interface{}, cacheManifest bool) {
+		time.Sleep(10 * time.Microsecond)
+		return nil, false
+	}
+
+	t.ResetTimer()
+
+	for n := 0; n < t.N; n++ {
+		err := c.sync()
+		require.NoError(t, err)
+	}
+}
+
 func TestEnsureSynced(t *testing.T) {
 	obj1 := &appsv1.Deployment{
 		TypeMeta: metav1.TypeMeta{