VAI: Replace namespace+name default sort with id (#724)

* Sort by ID rather than metadata.namespace,metadata.name

- In tests add an id field to the list of fields to cache/create and to the objects.

* Add benchmark tests to compare sorting by ns/name vs id.

pkg/sqlcache/informer/listoption_indexer.go

@@ -794,7 +794,8 @@ func (l *ListOptionIndexer) constructQuery(lo *sqltypes.ListOptions, partitions
 	} else {
 		// make sure one default order is always picked
 		if l.namespaced {
-			query += "\n  ORDER BY f.\"metadata.namespace\" ASC, f.\"metadata.name\" ASC "
+			// ID == metadata.namespace + "/" + metaqata.name
+			query += "\n  ORDER BY f.\"id\" ASC "
 		} else {
 			query += "\n  ORDER BY f.\"metadata.name\" ASC "
 		}

pkg/sqlcache/informer/listoption_indexer_test.go

@@ -11,6 +11,7 @@ import (
 	"database/sql"
 	"errors"
 	"fmt"
+	"math"
 	"os"
 	"testing"
 	"time"
@@ -54,6 +55,15 @@ func makeListOptionIndexer(ctx context.Context, opts ListOptionIndexerOptions, s
 	if err != nil {
 		return nil, "", err
 	}
+	if opts.IsNamespaced {
+		// Can't use slices.Compare because []string doesn't implement comparable
+		idEntry := []string{"id"}
+		if opts.Fields == nil {
+			opts.Fields = [][]string{idEntry}
+		} else {
+			opts.Fields = append(opts.Fields, idEntry)
+		}
+	}
 
 	listOptionIndexer, err := NewListOptionIndexer(ctx, s, opts)
 	if err != nil {
@@ -1105,6 +1115,145 @@ func TestNewListOptionIndexerEasy(t *testing.T) {
 	}
 }
 
+func makePseudoRandomList(size int) *unstructured.UnstructuredList {
+	numLength := 1 + int(math.Floor(math.Log10(float64(size))))
+	name_template := fmt.Sprintf("n%%0%dd", numLength)
+	// Make a predictable but randomish list of numbers
+	// item 0: ns0, n0
+	// item 23: ns0, n1
+	// item 46: ns0, n2
+	// At some point the index will be set back to the start
+	// the ns value goes up every <ns_delta> hits
+	// the name_val is the index, and i provides the name-value as we walk through the array.
+	// Use any size, as long as both name_delta (23) and ns_delta (17) are relatively prime to it.
+	// This assures that every index in the array will be initialized to an actual object
+	name_val := 0
+	name_delta := 23 // space the names out in runs of 23
+
+	ns_val := 0
+	ns_block := 0
+	ns_delta := 17 // so only 17 namespaces
+	namespace_template := "ns%02d"
+
+	items := make([]unstructured.Unstructured, size)
+	for i := range size {
+		nv := fmt.Sprintf(name_template, i)
+		nsv := fmt.Sprintf(namespace_template, ns_block)
+		obj := unstructured.Unstructured{
+			Object: map[string]any{
+				"metadata": map[string]any{
+					"name":      nv,
+					"namespace": nsv,
+				},
+				"id": nv + "/" + nsv,
+			},
+		}
+		items[name_val] = obj
+		name_val += name_delta
+		if name_val >= size {
+			name_val -= size
+		}
+		ns_val += ns_delta
+		if ns_val >= size {
+			ns_val -= size
+			ns_block += 1
+		}
+	}
+	ulist := &unstructured.UnstructuredList{
+		Items: items,
+	}
+	gvk := schema.GroupVersionKind{
+		Group:   "",
+		Version: "v1",
+		Kind:    "ConfigMap",
+	}
+	ulist.SetGroupVersionKind(gvk)
+	return ulist
+}
+
+func verifyListIsSorted(b *testing.B, list *unstructured.UnstructuredList, size int) {
+	for i := range size - 1 {
+		curr := list.Items[i]
+		next := list.Items[i+1]
+		if curr.GetNamespace() == next.GetNamespace() {
+			assert.Less(b, curr.GetName(), next.GetName())
+		} else {
+			assert.Less(b, curr.GetNamespace(), next.GetNamespace())
+		}
+	}
+}
+func BenchmarkNamespaceNameList(b *testing.B) {
+	// At 50,000,000 this starts to get very slow
+	size := 10000
+	itemList := makePseudoRandomList(size)
+	ctx := context.Background()
+	opts := ListOptionIndexerOptions{
+		IsNamespaced: true,
+	}
+	loi, dbPath, err := makeListOptionIndexer(ctx, opts, false)
+	defer cleanTempFiles(dbPath)
+	assert.NoError(b, err)
+	for _, item := range itemList.Items {
+		err = loi.Add(&item)
+		assert.NoError(b, err)
+	}
+	b.Run(fmt.Sprintf("sort-%d with explicit namespace/name", size), func(b *testing.B) {
+		listOptions := sqltypes.ListOptions{
+			SortList: sqltypes.SortList{
+				SortDirectives: []sqltypes.Sort{
+					{
+						Fields: []string{"metadata", "namespace"},
+						Order:  sqltypes.ASC,
+					},
+					{
+						Fields: []string{"metadata", "name"},
+						Order:  sqltypes.ASC,
+					},
+				},
+			},
+		}
+		partitions := []partition.Partition{{All: true}}
+		ns := ""
+		list, total, _, err := loi.ListByOptions(ctx, &listOptions, partitions, ns)
+		if err != nil {
+			b.Fatal("error getting data", err)
+		}
+		if total != size {
+			b.Errorf("expecting %d items, got %d", size, total)
+		}
+		if len(list.Items) != size {
+			b.Errorf("expecting %d items, got %d", size, len(list.Items))
+		}
+		//verifyListIsSorted(b, list, size)
+	})
+	b.Run(fmt.Sprintf("sort-%d with explicit id", size), func(b *testing.B) {
+		listOptions := sqltypes.ListOptions{
+			SortList: sqltypes.SortList{
+				SortDirectives: []sqltypes.Sort{
+					{
+						Fields: []string{"id"},
+						Order:  sqltypes.ASC,
+					},
+				},
+			},
+		}
+		partitions := []partition.Partition{{All: true}}
+		ns := ""
+		list, total, _, err := loi.ListByOptions(ctx, &listOptions, partitions, ns)
+		if err != nil {
+			b.Fatal("error getting data", err)
+		}
+		if total != size {
+			b.Errorf("expecting %d items, got %d", size, total)
+		}
+		if len(list.Items) != size {
+			b.Errorf("expecting %d items, got %d", size, len(list.Items))
+		}
+		//verifyListIsSorted(b, list, size)
+	})
+
+}
+
 func TestUserDefinedExtractFunction(t *testing.T) {
 	makeObj := func(name string, barSeparatedHosts string) map[string]any {
 		h1 := map[string]any{
@@ -2727,6 +2876,7 @@ func TestWatchResourceVersion(t *testing.T) {
 	foo.SetResourceVersion("100")
 	foo.SetName("foo")
 	foo.SetNamespace("foo")
+	foo.Object["id"] = "foo/foo"
 	foo.SetLabels(map[string]string{
 		"app": "foo",
 	})
@@ -2741,6 +2891,7 @@ func TestWatchResourceVersion(t *testing.T) {
 	bar.SetResourceVersion("150")
 	bar.SetName("bar")
 	bar.SetNamespace("bar")
+	bar.Object["id"] = "bar/bar"
 	bar.SetLabels(map[string]string{
 		"app": "bar",
 	})
@@ -3030,6 +3181,7 @@ func TestNonNumberResourceVersion(t *testing.T) {
 			"metadata": map[string]any{
 				"name": "foo",
 			},
+			"id": "/foo",
 		},
 	}
 	foo.SetResourceVersion("a")
@@ -3043,6 +3195,7 @@ func TestNonNumberResourceVersion(t *testing.T) {
 			"metadata": map[string]any{
 				"name": "bar",
 			},
+			"id": "/bar",
 		},
 	}
 	bar.SetResourceVersion("c")
@@ -3062,6 +3215,6 @@ func TestNonNumberResourceVersion(t *testing.T) {
 	require.NoError(t, err)
 
 	list, _, _, err := loi.ListByOptions(ctx, &sqltypes.ListOptions{}, []partition.Partition{{All: true}}, "")
-	assert.NoError(t, err)
+	require.NoError(t, err)
 	assert.Equal(t, expectedList.Items, list.Items)
 }

pkg/sqlcache/integration_test.go

@@ -61,7 +61,7 @@ func (i *IntegrationSuite) TearDownSuite() {
 }
 
 func (i *IntegrationSuite) TestSQLCacheFilters() {
-	fields := [][]string{{"metadata", "annotations", "somekey"}}
+	fields := [][]string{{"id"}, {"metadata", "annotations", "somekey"}}
 	require := i.Require()
 	configMapWithAnnotations := func(name string, annotations map[string]string) v1.ConfigMap {
 		return v1.ConfigMap{