Index source targets instead of scanning every mapping per resource

Rebuilding the registry view caches resolved each resource's metrics target by scanning every bySource mapping, an O(resources x mappings) pass holding the write lock. At about five thousand resources that is tens of millions of map iterations per rebuild, and rebuilds run on every poll cycle. The rebuild now inverts the mappings once and the cached index also serves single-resource lookups while the views are clean, with the legacy scan kept for dirty-state callers. An equivalence test pins index results to the legacy scan. The agent-parent fallback scan also ran its per-entry string canonicalization before the type check, so every guest in the estate paid allocation cost in a scan that only ever matches agents. The cheap checks now run first. Contract-Neutral: performance-only rework of metrics-target resolution and scan ordering, resolved values are pinned unchanged by equivalence tests
2026-09-11 14:00:29 +00:00 · 2026-08-16 21:48:31 +01:00
parent 90081dacb4
commit fc50f61cdb
2 changed files with 177 additions and 9 deletions
@@ -120,6 +120,11 @@ type ResourceRegistry struct {
 	cachedK8sDeployments   []*K8sDeploymentView
 	cachedWorkload         []*WorkloadView
 	cachedInfra            []*InfrastructureView
+	// cachedSourceTargets inverts bySource (resource ID -> its source
+	// entries) so metrics-target resolution is O(own entries) instead of a
+	// scan over every mapping. Rebuilt with the views; valid only while
+	// viewsDirty is false, like every other cached field above.
+	cachedSourceTargets map[string][]SourceTarget
 }

 // NewRegistry creates a new registry using the provided store for overrides.
@@ -1455,6 +1460,23 @@ func (rr *ResourceRegistry) metricsTargetForResourceLocked(resourceID string) *M
 		return nil
 	}

+	// The inverse index shares the view-cache lifecycle: every bySource
+	// mutation runs inside a batch update that ends with viewsDirty = true,
+	// so a clean flag means the index still matches the mappings. While
+	// dirty, fall back to the legacy full scan.
+	var sourceTargets []SourceTarget
+	if !rr.viewsDirty && rr.cachedSourceTargets != nil {
+		sourceTargets = rr.cachedSourceTargets[resourceID]
+	} else {
+		sourceTargets = rr.collectSourceTargetsLocked(resourceID, resource.Type)
+	}
+
+	return rr.metricsTargetFromSourceTargets(resource, sourceTargets)
+}
+
+// collectSourceTargetsLocked is the legacy O(all mappings) scan for one
+// resource, used only while the cached inverse index is invalid.
+func (rr *ResourceRegistry) collectSourceTargetsLocked(resourceID string, resourceType ResourceType) []SourceTarget {
 	sourceTargets := make([]SourceTarget, 0)
 	for source, mapping := range rr.bySource {
 		for sourceID, mappedID := range mapping {
@@ -1462,17 +1484,38 @@ func (rr *ResourceRegistry) metricsTargetForResourceLocked(resourceID string) *M
 				continue
 			}
 			sourceTargets = append(sourceTargets, SourceTarget{
+				Source:      source,
+				SourceID:    sourceID,
+				CandidateID: rr.sourceSpecificID(resourceType, source, sourceID),
+			})
+		}
+	}
+	return sourceTargets
+}
+
+// buildSourceTargetsIndexLocked inverts every bySource mapping in one pass.
+func (rr *ResourceRegistry) buildSourceTargetsIndexLocked() map[string][]SourceTarget {
+	index := make(map[string][]SourceTarget, len(rr.resources))
+	for source, mapping := range rr.bySource {
+		for sourceID, mappedID := range mapping {
+			resource := rr.resources[mappedID]
+			if resource == nil {
+				continue
+			}
+			index[mappedID] = append(index[mappedID], SourceTarget{
 				Source:      source,
 				SourceID:    sourceID,
 				CandidateID: rr.sourceSpecificID(resource.Type, source, sourceID),
 			})
 		}
 	}
+	return index
+}

+func (rr *ResourceRegistry) metricsTargetFromSourceTargets(resource *Resource, sourceTargets []SourceTarget) *MetricsTarget {
 	if target := BuildMetricsTarget(*resource, sourceTargets); target != nil {
 		return target
 	}
-
 	return cloneMetricsTarget(resource.MetricsTarget)
 }

@@ -4343,19 +4386,23 @@ func (rr *ResourceRegistry) proxmoxNodeParentIDFromResourcesLocked(instance, clu
 	bestID := ""
 	bestScore := -1
 	for id, resource := range rr.resources {
-		if resource == nil {
+		if resource == nil || resource.Proxmox == nil {
+			continue
+		}
+		// Type and node-name checks run before the ID canonicalization:
+		// in a large estate almost every entry is a guest, and this scan
+		// runs per guest, so per-entry string work here is the difference
+		// between a cheap pass and an O(n^2) allocation storm.
+		if CanonicalResourceType(resource.Type) != ResourceTypeAgent {
+			continue
+		}
+		if !strings.EqualFold(strings.TrimSpace(resource.Proxmox.NodeName), nodeName) {
 			continue
 		}
 		candidateID := CanonicalResourceID(strings.TrimSpace(id))
 		if candidateID == "" || candidateID == excludeID {
 			continue
 		}
-		if CanonicalResourceType(resource.Type) != ResourceTypeAgent || resource.Proxmox == nil {
-			continue
-		}
-		if !strings.EqualFold(strings.TrimSpace(resource.Proxmox.NodeName), nodeName) {
-			continue
-		}
 		score := proxmoxNodeParentScopeScore(instance, clusterName, resource.Proxmox)
 		if score < 0 {
 			continue
@@ -5404,9 +5451,15 @@ func (rr *ResourceRegistry) rebuildViews() {
 	rr.cachedWorkload = nil
 	rr.cachedInfra = nil

+	// One O(mappings) pass instead of a per-resource scan over every
+	// mapping: at thousands of resources the difference is seconds of
+	// write-lock hold time per rebuild.
+	sourceTargetsIndex := rr.buildSourceTargetsIndexLocked()
+	rr.cachedSourceTargets = sourceTargetsIndex
+
 	for _, r := range rr.resources {
 		viewResource := cloneResourcePtr(r)
-		viewResource.MetricsTarget = rr.metricsTargetForResourceLocked(r.ID)
+		viewResource.MetricsTarget = rr.metricsTargetFromSourceTargets(r, sourceTargetsIndex[r.ID])
 		switch r.Type {
 		case ResourceTypeVM:
 			v := NewVMView(viewResource)
@@ -0,0 +1,115 @@
+package unifiedresources
+
+import (
+	"reflect"
+	"testing"
+	"time"
+
+	"github.com/rcourtman/pulse-go-rewrite/internal/models"
+)
+
+func sourceTargetsFixtureRegistry() *ResourceRegistry {
+	rr := NewRegistry(nil)
+	now := time.Date(2026, 8, 16, 10, 0, 0, 0, time.UTC)
+
+	rr.IngestSnapshot(models.StateSnapshot{
+		Nodes: []models.Node{
+			{
+				ID:              "mock-cluster-pve1",
+				NodeIdentity:    "mock-cluster-pve1",
+				Name:            "pve1",
+				Instance:        "Core Fabric",
+				ClusterName:     "Core Fabric",
+				IsClusterMember: true,
+				Status:          "online",
+				LastSeen:        now,
+			},
+			{
+				ID:              "mock-cluster-pve2",
+				NodeIdentity:    "mock-cluster-pve2",
+				Name:            "pve2",
+				Instance:        "Core Fabric",
+				ClusterName:     "Core Fabric",
+				IsClusterMember: true,
+				Status:          "online",
+				LastSeen:        now,
+			},
+		},
+		VMs: []models.VM{
+			{ID: "Core Fabric:pve1:100", VMID: 100, Name: "web-01", Node: "pve1", Instance: "Core Fabric", Status: "running", LastSeen: now},
+			{ID: "Core Fabric:pve2:101", VMID: 101, Name: "db-01", Node: "pve2", Instance: "Core Fabric", Status: "running", LastSeen: now},
+		},
+		Containers: []models.Container{
+			{ID: "Core Fabric:pve1:104", VMID: 104, Name: "auth-01", Node: "pve1", Instance: "Core Fabric", Status: "running", LastSeen: now},
+		},
+	})
+	return rr
+}
+
+// The cached inverse index must resolve exactly the same metrics targets as
+// the legacy per-resource scan over every bySource mapping.
+func TestMetricsTargetIndexMatchesLegacyScan(t *testing.T) {
+	rr := sourceTargetsFixtureRegistry()
+
+	ids := make([]string, 0)
+	for _, r := range rr.List() {
+		ids = append(ids, r.ID)
+	}
+	if len(ids) < 5 {
+		t.Fatalf("fixture too small: %d resources", len(ids))
+	}
+
+	// While views are dirty the legacy scan answers.
+	legacy := make(map[string]*MetricsTarget, len(ids))
+	rr.mu.Lock()
+	if !rr.viewsDirty {
+		rr.viewsDirty = true
+	}
+	rr.cachedSourceTargets = nil
+	for _, id := range ids {
+		legacy[id] = rr.metricsTargetForResourceLocked(id)
+	}
+	rr.mu.Unlock()
+
+	// A view read rebuilds the caches, activating the index path.
+	rr.VMs()
+	rr.mu.RLock()
+	if rr.viewsDirty {
+		rr.mu.RUnlock()
+		t.Fatal("views still dirty after rebuild; index path not active")
+	}
+	if rr.cachedSourceTargets == nil {
+		rr.mu.RUnlock()
+		t.Fatal("cachedSourceTargets not built by rebuildViews")
+	}
+	indexed := make(map[string]*MetricsTarget, len(ids))
+	for _, id := range ids {
+		indexed[id] = rr.metricsTargetForResourceLocked(id)
+	}
+	rr.mu.RUnlock()
+
+	for _, id := range ids {
+		if !reflect.DeepEqual(legacy[id], indexed[id]) {
+			t.Fatalf("metrics target diverged for %s:\nlegacy:  %+v\nindexed: %+v", id, legacy[id], indexed[id])
+		}
+	}
+}
+
+// View-embedded metrics targets must match what the public per-ID API
+// resolves for the same resource.
+func TestRebuiltViewMetricsTargetsMatchPublicAPI(t *testing.T) {
+	rr := sourceTargetsFixtureRegistry()
+
+	for _, vm := range rr.VMs() {
+		fromAPI := BuildMetricsTargetForRegistry(rr, vm.ID())
+		if !reflect.DeepEqual(vm.MetricsTarget(), fromAPI) {
+			t.Fatalf("VM %s view target %+v != API target %+v", vm.ID(), vm.MetricsTarget(), fromAPI)
+		}
+	}
+	for _, ct := range rr.Containers() {
+		fromAPI := BuildMetricsTargetForRegistry(rr, ct.ID())
+		if !reflect.DeepEqual(ct.MetricsTarget(), fromAPI) {
+			t.Fatalf("container %s view target %+v != API target %+v", ct.ID(), ct.MetricsTarget(), fromAPI)
+		}
+	}
+}