diff --git a/docs/release-control/v6/internal/subsystems/agent-lifecycle.md b/docs/release-control/v6/internal/subsystems/agent-lifecycle.md index 2625c04957..8847722e10 100644 --- a/docs/release-control/v6/internal/subsystems/agent-lifecycle.md +++ b/docs/release-control/v6/internal/subsystems/agent-lifecycle.md @@ -9459,3 +9459,20 @@ pair the operator has decided about since. The agents API's unlink is unchanged. Registration, enrolment, install, update, removal, token binding and report identity are unchanged. The unified-resources contract records the split's rules. + +### Host-agent evaluation lists nodes only when an offline host needs them + +`Monitor.evaluateHostAgents` (`internal/monitoring/monitor_agents.go`) runs on +every monitor tick. It used to list every Proxmox node through +`NodesSnapshot` before looking at a single host, only to give an offline host +its shared-system alert correlation (`sharedSystemAlertCorrelationForHost`, +which reads node ID, linked agent and instance). It now lists the nodes when +the first offline host that names a linked node asks, through +`structureNodes`, which in mock mode serves the view built for the current +fixture structure instead of rebuilding the estate after each metric tick. The +liveness window, the online and offline transitions, the host lifecycle, the +telemetry expiry and the alert correlation itself are unchanged, and no agent +report, enrollment, lease or command path is touched +(`TestSharedSystemAlertCorrelationForHostLazyListsNodesOnDemand` and +`TestMockStructureConsumersDoNotRebuildTheViewAfterFixtureTicks` in +`internal/monitoring/monitor_host_agents_test.go`). diff --git a/docs/release-control/v6/internal/subsystems/api-contracts.md b/docs/release-control/v6/internal/subsystems/api-contracts.md index 3ea9c617e3..79ab1face9 100644 --- a/docs/release-control/v6/internal/subsystems/api-contracts.md +++ b/docs/release-control/v6/internal/subsystems/api-contracts.md @@ -12080,3 +12080,22 @@ it. The request, status codes, response shape and the meaning of an empty decision is added: `exclusions` still counts one link pair each. `TestResourceReportMergeSourceFilterDetachesChainMembers` reports a chain through the handler and the REST listing. + +### The connection-degraded alert feed reads platform rows only + +`buildAlertConnectionSnapshotsWithRuntimeSources` (`internal/api/connections_alerts.go`) +feeds the connection-degraded alert, and `snapshotConnectionsForAlerts` keeps +only the PVE, PBS, PMG, VMware and TrueNAS rows. It now builds the aggregator's +inputs with `aggregatorRuntimeSources.platformRowsOnly`, which leaves out the +monitor's hosts, their desired-config fingerprints and the PBS reported node +names: they only feed agent rows, which that snapshot drops, and a PBS host +alias, which it does not carry. The connections endpoint, diagnostics and the +runtime inventory sources still read them. No route, request, response, status +code, header or payload changes, and a platform row is the same with and +without those reads (`TestAlertConnectionSnapshotsMatchTheFullAggregatorOnPlatformRows` +and `TestAlertConnectionSnapshotsKeepConfiguredPlatformRowsWithoutTheHosts` in +`internal/api/runtime_inventory_sources_test.go`). The demo Patrol resolver +(`patrolResourceOperatorStateProvider` in `internal/api/router.go`) resolves a +finding's resource reference through `Monitor.GetUnifiedStructureReadState`; in +mock mode that is the view built for the current fixture structure, outside it +the read state it used before. diff --git a/docs/release-control/v6/internal/subsystems/monitoring.md b/docs/release-control/v6/internal/subsystems/monitoring.md index d7ee52daa5..53043e47c1 100644 --- a/docs/release-control/v6/internal/subsystems/monitoring.md +++ b/docs/release-control/v6/internal/subsystems/monitoring.md @@ -3634,9 +3634,34 @@ being reconfigured or a link change rebuilds the view on the next lookup; as before, two lookups that miss together can each build one, and a cold or invalidated cache builds even for an ID the estate does not list. Once the view is current an unknown ID resolves to nothing and falls to the raw -resource store. The view stays keyed on the data version for every other -consumer (broadcast, `/api/state`, `GetUnifiedReadStateOrSnapshot`), which -still rebuild it on a data-version miss. +resource store. The view stays keyed on the data version for the consumers +that read what a tick moves (broadcast, `/api/state`, +`GetUnifiedReadStateOrSnapshot`), which still rebuild it on a data-version +miss. + +Callers that read the estate's identity and topology only share the +structure-keyed view instead (`currentStructureUnifiedStateView`, with +`GetUnifiedStructureReadState` and `currentModeStructureReadState` over it in +`internal/monitoring/unified_structure_view.go`): the Docker-alert prune +(`pruneStaleDockerAlerts`, host IDs), the host-agent evaluation +(`evaluateHostAgents`, which runs on every monitor tick and lists nodes through +`structureNodes`, for their IDs and links, only when an offline host names a +linked node), the chart cache prewarm (metric IDs) and the demo patrol's +operator-state resolver (`patrolResourceOperatorStateProvider`, canonical ID +resolution). Such a caller can be handed a view several ticks old, so it must +not read status, metric values, timestamps, sensors or anything a link merges by +freshness, and a new consumer that does belongs on the data-version view. The +mock storage chart history (`resolveMockStorageTotal`) is one of those: a linked +pool takes its capacity from whichever source was seen last, which moves +without a structure change, so it totals each pool whose chart is not cached +from the data-version view, and `GetStorageMetricsForChartBatch` reads that view +once for all the pools of a request instead of once per pool +(`TestMockStorageChartReadsTheEstateOnlyForAnUncachedChart`). The connection-degraded alert feed +(`buildAlertConnectionSnapshotsWithRuntimeSources`) reads neither the hosts nor +the PBS instances: it keeps the platform rows and drops every agent row, and a +PBS row's reported node name only becomes a host alias that the alert snapshot +does not carry, so it asks the aggregator for platform rows only +(`aggregatorRuntimeSources.platformRowsOnly`). The revision is read before the data version when a view is cached, and the fixture advances its data version before its revision @@ -3658,16 +3683,36 @@ busy loops missed the view cache on 675 of 929 requests at a 16-second average build and had not served by 606 s; with this change it served at 148 s (13 s on the core alone). This was not a regression: builds from 1a094646d7 and d2ac0c5112 stall identically under equal contention and both start in 14 to -16 seconds at normal load. The remaining time on a starved host is the other -consumers of the shared view, which rebuild it whenever they find it stale (on -the starved run, 42 view requests in a start of 194 s were all misses, from the -Docker-alert prune, the connection snapshots and the host-agent evaluation), -and the monitor's repeated full resource-store passes at startup. Loops that -still reach the data-version view once per item are unchanged: the mock storage -chart history (`resolveMockStorageTotal` per uncached pool), the demo patrol's -finding adds (`patrolResourceOperatorStateProvider`) and, with -`PULSE_MOCK_KEEP_REAL_POLLING`, the per-node polling lookups -(`linkedHostForNode`, `getHostAgentTemperatureForNode`). +16 seconds at normal load. The remaining time on a starved host was the other +consumers of the shared view, which rebuilt it whenever they found it stale (on +the starved run, 42 view requests in a start of 194 s were all misses: the +Docker-alert prune 18, the connection snapshots 13, the host-agent evaluation 8, +the chart prewarm 2 and the metrics-target lookup 1), and the monitor's repeated full resource-store +passes at startup. The consumers above now ride the structure view or skip the +read, and the passes are one (see "Store and provider wiring publishes the +estate once, and only when it changed"). The loops that still reach the +data-version view once per item are the per-node polling lookups of a monitor +that keeps real polling in mock mode (`PULSE_MOCK_KEEP_REAL_POLLING`): +`linkedHostForNode`, `getHostAgentTemperatureForNode` and +`carriedTemperatureOutlivesAgentLease`. They read a host's status, disks, +sensors and report lease, which a tick moves, so the structure view cannot +answer them; hoisting the read out of the per-node loop would need the read +state threaded through the node poller, and they run in the polling goroutines +after startup, not before the listener opens. + +Measured on pulse-dev (single runs on a shared host at load 50 to 90, the +process pinned to one core with GOMAXPROCS=1, probe builds counting estate +builds): before the listener answered, a start of the previous build made 19 +store passes and 25 estate view builds; with the wiring change alone it made one +pass and two builds (the first metrics-target lookup and the Docker-alert +prune); with the structure view as well it makes one pass and one build. With no +client connected, a mock-mode process kept building the estate about once every +nine seconds (33 builds in the 299-second life of a run with the wiring change +alone) and used 12.3 seconds of CPU over a 180-second window; with the +structure view it built the estate twice in all, both at startup, and used 3.3 +seconds over the same window. The previous build used 9.0 seconds over that +window in a separate run, so read the 12.3 against 3.3 as the comparison, not as +exact percentages. `TestMockMetricsTargetLookupsDoNotRebuildTheViewAfterFixtureTicks` and `TestMockMetricsTargetForUnknownIDDoesNotRebuildTheViewAfterFixtureTicks` fail on diff --git a/docs/release-control/v6/internal/subsystems/performance-and-scalability.md b/docs/release-control/v6/internal/subsystems/performance-and-scalability.md index a7372f932d..eeb88823d2 100644 --- a/docs/release-control/v6/internal/subsystems/performance-and-scalability.md +++ b/docs/release-control/v6/internal/subsystems/performance-and-scalability.md @@ -3748,9 +3748,18 @@ alert loop, which is what the one-off measurement above showed. Run both benchmarks with `-benchtime=3x`, since the registry case rebuilds its ingest outside the timer. `TestMockMetricsTargetLookupsDoNotRebuildTheViewAfterFixtureTicks` is the guard that fails on a rebuild per lookup. The remaining startup cost -under starvation is the monitor's repeated full resource-store passes (one per -supplemental provider registration) and the other consumers of the shared view, -which rebuild it whenever they find it stale; neither is changed here. +under starvation was the monitor's repeated full resource-store passes (one per +supplemental provider registration, 19 before the listener answered) and the +other consumers of the shared view, which rebuilt it whenever they found it +stale. A start now makes one pass and one estate build, and a mock-mode process +with no client connected no longer builds the estate on every monitor tick (the +data-version consumers, broadcast and `/api/state`, still rebuild it after a +tick for a connected client). The callers that read +identity and topology only share the view built for the current fixture +structure, and the connection-degraded alert feed no longer asks the monitor for +hosts or PBS instances it throws away (see "Mock-mode metrics-target lookups +ride the fixture structure revision" in the monitoring contract for the callers +and the measurements). ### Update evidence reuses the bounded node observation diff --git a/docs/release-control/v6/internal/subsystems/security-privacy.md b/docs/release-control/v6/internal/subsystems/security-privacy.md index 39579156bc..268ff47963 100644 --- a/docs/release-control/v6/internal/subsystems/security-privacy.md +++ b/docs/release-control/v6/internal/subsystems/security-privacy.md @@ -3168,3 +3168,18 @@ that node's row in the same monitor. This only withholds or substitutes values the caller could already read; the new `NodeView` and `DockerHostView` `SourceStatus` accessors expose no field beyond the delivery state the registry already records on the resource. + +### Patrol operator-state resolution keeps its org scoping + +`patrolResourceOperatorStateProvider` (`internal/api/router.go`) resolves a +finding's resource reference to a canonical ID before it reads the operator +state, and now reads the monitor's structure read state +(`Monitor.GetUnifiedStructureReadState`) for that. The org is resolved by +`resolveMonitorForOrg` as before, the operator-state store is the org's own, and +the resolver is identity-only. In mock mode the read state can be several metric +ticks old; the fixture moves no pod between nodes and re-IDs nothing on a tick +(pod node names are set when the estate is built, and they feed the platform +aliases a reference can resolve through), so a reference resolves to the same +canonical ID for as long as the fixture structure holds. Outside mock mode it +is the read state the resolver used before. No authentication, authorization, token or +tenant-isolation path is touched. diff --git a/docs/release-control/v6/internal/subsystems/storage-recovery.md b/docs/release-control/v6/internal/subsystems/storage-recovery.md index a1b9dc254c..ab402ed7fe 100644 --- a/docs/release-control/v6/internal/subsystems/storage-recovery.md +++ b/docs/release-control/v6/internal/subsystems/storage-recovery.md @@ -6784,3 +6784,13 @@ excludes the links that join a member carrying the named source to the rest of the merged resource. That changes which pairs the report excludes, through the same exclusion write as before; no schema, backup, retention, migration or recovery path is added or moved. + +### Mock-mode structure views open no storage or recovery path + +`internal/api/connections_alerts.go` and `internal/api/router.go` now ask the +monitor for less in mock mode: the connection-degraded alert feed no longer +reads the hosts and PBS instances it discards, and the demo Patrol resolver +reads the structure view (see "Mock-mode metrics-target lookups ride the +fixture structure revision" in the monitoring contract). Real mode, its stores, +backups, retention, snapshots, migrations and recovery paths are unchanged, and +none is added or moved. diff --git a/internal/api/connections_alerts.go b/internal/api/connections_alerts.go index 0a8dbdbf41..acd8c948d3 100644 --- a/internal/api/connections_alerts.go +++ b/internal/api/connections_alerts.go @@ -15,6 +15,12 @@ type aggregatorRuntimeSources struct { orgID string vmwarePoller *monitoring.VMwarePoller truenasPoller *monitoring.TrueNASPoller + // platformRowsOnly leaves out what only the agent rows and the PBS host + // aliases read: the monitor's hosts and PBS instances. A caller that keeps + // the platform rows alone (the connection-degraded alert feed) would + // otherwise ask the monitor for both, and in mock mode each ask rebuilds + // the whole estate view whenever a metric tick has made it stale. + platformRowsOnly bool } func buildAggregatorInputsWithRuntimeSources( @@ -61,11 +67,15 @@ func buildAggregatorInputsWithRuntimeSources( } if monitor != nil { - inputs.hosts = monitor.HostsSnapshot() - inputs.agentDesiredConfigs = connectionAgentDesiredConfigFingerprints(monitor, inputs.hosts, inputs.apiTokens) + if runtime.platformRowsOnly { + inputs.hosts = []models.Host{} + } else { + inputs.hosts = monitor.HostsSnapshot() + inputs.agentDesiredConfigs = connectionAgentDesiredConfigFingerprints(monitor, inputs.hosts, inputs.apiTokens) + inputs.pbsReportedNodeNames = pbsReportedNodeNamesByInstance(monitor.PBSInstancesSnapshot()) + } inputs.instanceHealth = instanceHealthByKey(monitor.SchedulerHealth()) inputs.availabilityStatuses = monitor.AvailabilityStatusSnapshot() - inputs.pbsReportedNodeNames = pbsReportedNodeNamesByInstance(monitor.PBSInstancesSnapshot()) // A non-default org's cfg is its monitor's detached config copy // (#1619), which a settings save does not update: saved PBS and PMG // intervals reach the monitor only as runtime overrides. Scale the @@ -155,6 +165,10 @@ func buildAlertConnectionSnapshotsWithRuntimeSources( monitor *monitoring.Monitor, runtime aggregatorRuntimeSources, ) []alerts.ConnectionSnapshot { + // snapshotConnectionsForAlerts keeps the platform rows (PVE, PBS, PMG, + // VMware, TrueNAS) and drops every agent row, and a PBS row's reported node + // name only becomes a host alias that the snapshot does not carry. + runtime.platformRowsOnly = true inputs := buildAggregatorInputsWithRuntimeSources(ctx, cfg, persistence, monitor, runtime) return snapshotConnectionsForAlerts(buildConnections(inputs)) } diff --git a/internal/api/router.go b/internal/api/router.go index 30c3971cb5..fe2766d9e5 100644 --- a/internal/api/router.go +++ b/internal/api/router.go @@ -2242,7 +2242,7 @@ func (r *Router) patrolResourceOperatorStateProvider(orgID string) ai.ResourceOp return ai.ResourceOperatorStateProjection{}, false } canonicalID := resourceRef - if resolver, ok := r.resolveMonitorForOrg(orgID).GetUnifiedReadStateOrSnapshot().(canonicalResourceIDResolver); ok { + if resolver, ok := r.resolveMonitorForOrg(orgID).GetUnifiedStructureReadState().(canonicalResourceIDResolver); ok { if resolvedID, resolved := resolver.ResolveCanonicalResourceID(resourceRef); resolved { canonicalID = resolvedID } diff --git a/internal/api/runtime_inventory_sources_test.go b/internal/api/runtime_inventory_sources_test.go index cdbf40953f..5b56c83c24 100644 --- a/internal/api/runtime_inventory_sources_test.go +++ b/internal/api/runtime_inventory_sources_test.go @@ -11,6 +11,7 @@ import ( "time" "github.com/rcourtman/pulse-go-rewrite/internal/config" + "github.com/rcourtman/pulse-go-rewrite/internal/mock" "github.com/rcourtman/pulse-go-rewrite/internal/monitoring" ) @@ -314,3 +315,149 @@ func TestRuntimeInventorySourcesHandlerRejectsNonGET(t *testing.T) { t.Fatalf("status = %d, want 405 (%s)", rec.Code, rec.Body.String()) } } + +// mockMonitorWithStaleView returns a mock-mode monitor whose estate view was +// built before the fixture last ticked, and the structure read state it holds. +func mockMonitorWithStaleView(t *testing.T) (*monitoring.Monitor, *config.Config) { + t.Helper() + previous := mock.IsMockEnabled() + previousConfig := mock.GetConfig() + estate := mock.DefaultConfig + estate.NodeCount = 4 + estate.UpdateInterval = time.Second + mock.SetMockConfig(estate) + if err := mock.SetEnabled(true); err != nil { + t.Fatalf("enable mock mode: %v", err) + } + t.Cleanup(func() { + _ = mock.SetEnabled(false) + mock.SetMockConfig(previousConfig) + _ = mock.SetEnabled(previous) + }) + + cfg := &config.Config{DataPath: t.TempDir()} + monitor, err := monitoring.New(cfg) + if err != nil { + t.Fatalf("new monitor: %v", err) + } + t.Cleanup(monitor.Stop) + + monitor.GetUnifiedReadStateOrSnapshot() + tick := mock.FixtureDataVersion() + deadline := time.Now().Add(5 * time.Minute) + for mock.FixtureDataVersion() <= tick { + if time.Now().After(deadline) { + t.Fatal("the mock fixture did not tick") + } + time.Sleep(20 * time.Millisecond) + } + return monitor, cfg +} + +// The connection-degraded alert feed keeps the platform rows (PVE, PBS, PMG, +// VMware, TrueNAS) and drops every agent row, so it has no use for the +// monitor's hosts or PBS instances. In mock mode each of those reads rebuilds +// the whole estate view once a metric tick has made it stale, and the feed +// runs on startup's critical path. +func TestAlertConnectionSnapshotsDoNotRebuildTheMockViewAfterFixtureTicks(t *testing.T) { + monitor, cfg := mockMonitorWithStaleView(t) + warm := monitor.GetUnifiedStructureReadState() + + buildAlertConnectionSnapshotsWithRuntimeSources(context.Background(), cfg, nil, monitor, aggregatorRuntimeSources{orgID: "default"}) + if monitor.GetUnifiedStructureReadState() != warm { + t.Fatal("the alert connection feed rebuilt the unified view after a metric tick; it reads no host or PBS instance") + } + + // Control: the connections endpoint does read them, and the test would + // pass vacuously if a rebuild left no trace. + buildAggregatorInputsWithRuntimeSources(context.Background(), cfg, nil, monitor, aggregatorRuntimeSources{orgID: "default"}) + if monitor.GetUnifiedStructureReadState() == warm { + t.Fatal("reading the hosts did not rebuild the stale view, so this test cannot see a rebuild") + } +} + +// Leaving the hosts and PBS instances out must not change a platform row. The +// lean inputs are the full ones with exactly what platformRowsOnly omits taken +// away, so a supplemental fixture tick between two reads cannot make the rows +// differ for a reason other than the omission. +func TestAlertConnectionSnapshotsMatchTheFullAggregatorOnPlatformRows(t *testing.T) { + monitor, cfg := mockMonitorWithStaleView(t) + + full := buildAggregatorInputsWithRuntimeSources(context.Background(), cfg, nil, monitor, aggregatorRuntimeSources{orgID: "default"}) + lean := buildAggregatorInputsWithRuntimeSources(context.Background(), cfg, nil, monitor, aggregatorRuntimeSources{orgID: "default", platformRowsOnly: true}) + + if len(full.hosts) == 0 { + t.Fatal("the mock estate lists no host, so the comparison proves nothing") + } + if len(lean.hosts) != 0 || lean.agentDesiredConfigs != nil || lean.pbsReportedNodeNames != nil { + t.Fatalf("platformRowsOnly kept hosts=%d desiredConfigs=%v pbsNodeNames=%v", len(lean.hosts), lean.agentDesiredConfigs, lean.pbsReportedNodeNames) + } + + stripped := full + stripped.hosts = lean.hosts + stripped.agentDesiredConfigs = nil + stripped.pbsReportedNodeNames = nil + want := snapshotConnectionsForAlerts(buildConnections(full)) + got := snapshotConnectionsForAlerts(buildConnections(stripped)) + if len(want) == 0 { + t.Fatal("the mock estate yields no platform connection") + } + if !reflect.DeepEqual(got, want) { + t.Fatalf("platform connections changed without the hosts:\n got %+v\nwant %+v", got, want) + } +} + +// The patrol resolver turns a finding's resource reference into a canonical ID. +// It reads identity only, so in mock mode it must not rebuild the estate view +// after a metric tick (it runs once per finding). +func TestPatrolOperatorStateResolverDoesNotRebuildTheMockViewAfterFixtureTicks(t *testing.T) { + monitor, cfg := mockMonitorWithStaleView(t) + warm := monitor.GetUnifiedStructureReadState() + + router := &Router{ + resourceHandlers: NewResourceHandlers(&config.Config{DataPath: cfg.DataPath}), + monitor: monitor, + } + t.Cleanup(router.ShutdownResourceStores) + provider := router.patrolResourceOperatorStateProvider("default") + provider.OperatorStateProjection("some-finding-resource", time.Now()) + + if monitor.GetUnifiedStructureReadState() != warm { + t.Fatal("the patrol operator-state resolver rebuilt the unified view after a metric tick; it only resolves identity") + } +} + +// Mock mode drops the configured PVE, PBS and PMG instances from the ledger, so +// their rows are checked here with mock mode off: the platform rows an alert +// is raised from are the same with and without the hosts and PBS reads. +func TestAlertConnectionSnapshotsKeepConfiguredPlatformRowsWithoutTheHosts(t *testing.T) { + previousMock := mock.IsMockEnabled() + if err := mock.SetEnabled(false); err != nil { + t.Fatalf("disable mock mode: %v", err) + } + t.Cleanup(func() { _ = mock.SetEnabled(previousMock) }) + monitor, err := monitoring.New(&config.Config{DataPath: t.TempDir()}) + if err != nil { + t.Fatalf("new monitor: %v", err) + } + t.Cleanup(monitor.Stop) + cfg := &config.Config{ + DataPath: t.TempDir(), + PVEInstances: []config.PVEInstance{{Name: "pve-lab", Host: "https://pve-lab.example:8006"}}, + PBSInstances: []config.PBSInstance{{Name: "pbs-lab", Host: "https://pbs-lab.example:8007"}}, + PMGInstances: []config.PMGInstance{{Name: "pmg-lab", Host: "https://pmg-lab.example:8006"}}, + } + + full := buildAggregatorInputsWithRuntimeSources(context.Background(), cfg, nil, monitor, aggregatorRuntimeSources{orgID: "default"}) + lean := buildAggregatorInputsWithRuntimeSources(context.Background(), cfg, nil, monitor, aggregatorRuntimeSources{orgID: "default", platformRowsOnly: true}) + lean.now = full.now + + want := snapshotConnectionsForAlerts(buildConnections(full)) + got := snapshotConnectionsForAlerts(buildConnections(lean)) + if len(want) != 3 { + t.Fatalf("expected the PVE, PBS and PMG rows, got %+v", want) + } + if !reflect.DeepEqual(got, want) { + t.Fatalf("platform connections changed without the hosts:\n got %+v\nwant %+v", got, want) + } +} diff --git a/internal/monitoring/metrics_target_resolution.go b/internal/monitoring/metrics_target_resolution.go index 6d81f731a3..8d6f57f4b3 100644 --- a/internal/monitoring/metrics_target_resolution.go +++ b/internal/monitoring/metrics_target_resolution.go @@ -43,19 +43,7 @@ func (m *Monitor) mockMetricsTarget(resourceID string) (*unifiedresources.Metric return nil, false } - structure := mock.FixtureStructureRevision() - links := m.resourceStoreManualLinks() - - m.mockUnifiedViewMu.Lock() - view := m.mockUnifiedView - reusable := m.mockUnifiedViewValid && - m.mockUnifiedViewStructure == structure && - sameManualLinks(m.mockUnifiedViewLinks, links) - m.mockUnifiedViewMu.Unlock() - - if !reusable { - view = m.currentUnifiedStateView() - } + view := m.currentStructureUnifiedStateView() if view.metricsTargets == nil { return nil, true } diff --git a/internal/monitoring/mock_chart_history.go b/internal/monitoring/mock_chart_history.go index 507cd858ba..500e1ed165 100644 --- a/internal/monitoring/mock_chart_history.go +++ b/internal/monitoring/mock_chart_history.go @@ -6,6 +6,7 @@ import ( "time" "github.com/rcourtman/pulse-go-rewrite/internal/mock" + "github.com/rcourtman/pulse-go-rewrite/internal/unifiedresources" ) var ( @@ -171,9 +172,14 @@ func latestMetricPoint(points []MetricPoint) (MetricPoint, bool) { return latest, true } -func (m *Monitor) resolveMockStorageTotal(storageID string, inMemory map[string][]MetricPoint) float64 { - readState := m.GetUnifiedReadStateOrSnapshot() - if readState != nil { +// resolveMockStorageTotal totals one pool for a chart. readState lists the pools +// and is read here only if the pool's chart is not cached, so a request over +// many pools reads the estate view once (see GetStorageMetricsForChartBatch) +// and not once per pool, which on a starved process rebuilt the view for each. +// It is not the structure view: a linked pool takes its capacity from whichever +// source was seen last, and that moves without a structure change. +func (m *Monitor) resolveMockStorageTotal(storageID string, inMemory map[string][]MetricPoint, readState func() unifiedresources.ReadState) float64 { + if readState := readState(); readState != nil { for _, pool := range readState.StoragePools() { if pool == nil || strings.TrimSpace(pool.SourceID()) != strings.TrimSpace(storageID) { continue @@ -191,8 +197,8 @@ func (m *Monitor) resolveMockStorageTotal(storageID string, inMemory map[string] return 0 } -func (m *Monitor) mockStorageMetricsForChart(storageID string, duration time.Duration, inMemory map[string][]MetricPoint) map[string][]MetricPoint { - total := m.resolveMockStorageTotal(storageID, inMemory) +func (m *Monitor) mockStorageMetricsForChart(storageID string, duration time.Duration, inMemory map[string][]MetricPoint, readState func() unifiedresources.ReadState) map[string][]MetricPoint { + total := m.resolveMockStorageTotal(storageID, inMemory, readState) if total <= 0 { return cloneMetricPointMap(inMemory) } diff --git a/internal/monitoring/monitor.go b/internal/monitoring/monitor.go index 72aff4182a..a3daa80544 100644 --- a/internal/monitoring/monitor.go +++ b/internal/monitoring/monitor.go @@ -3610,7 +3610,10 @@ func (m *Monitor) NodesSnapshot() []models.Node { if m == nil { return nil } - readState := m.GetUnifiedReadStateOrSnapshot() + return nodesSnapshotFromReadState(m.GetUnifiedReadStateOrSnapshot()) +} + +func nodesSnapshotFromReadState(readState unifiedresources.ReadState) []models.Node { if readState == nil { return nil } diff --git a/internal/monitoring/monitor_agents.go b/internal/monitoring/monitor_agents.go index 0d93930105..7a391fc4a7 100644 --- a/internal/monitoring/monitor_agents.go +++ b/internal/monitoring/monitor_agents.go @@ -5227,7 +5227,15 @@ func hostAgentHealthWindow(interval int) time.Duration { // evaluateHostAgents updates health for host agents based on last report time. func (m *Monitor) evaluateHostAgents(now time.Time) { hosts := m.state.GetHosts() - nodes := m.NodesSnapshot() + // Only an offline host that names a linked node asks which node it is + // linked to, and only for the node's identity, so the nodes are listed when + // the first one does, from the structure view: this runs on every monitor + // tick, and in mock mode a listing of the data-version view rebuilds the + // whole estate after each metric tick. + nodes := sync.OnceValue(m.structureNodes) + offlineCorrelation := func(host models.Host) *alerts.AlertCorrelation { + return sharedSystemAlertCorrelationForHostLazy(host, nodes) + } liveHostIDs := make(map[string]struct{}, len(hosts)) resourceRefreshNeeded := false for _, host := range hosts { @@ -5279,7 +5287,7 @@ func (m *Monitor) evaluateHostAgents(now time.Time) { if m.alertManager != nil { m.alertManager.HandleHostOfflineWithCorrelation( hostCopy, - sharedSystemAlertCorrelationForHost(hostCopy, nodes), + offlineCorrelation(hostCopy), ) } } @@ -5304,7 +5312,7 @@ func (m *Monitor) evaluateHostAgents(now time.Time) { if m.alertManager != nil { m.alertManager.HandleHostOfflineWithCorrelation( host, - sharedSystemAlertCorrelationForHost(host, nodes), + offlineCorrelation(host), ) } } @@ -5314,6 +5322,15 @@ func (m *Monitor) evaluateHostAgents(now time.Time) { } } +// sharedSystemAlertCorrelationForHostLazy is sharedSystemAlertCorrelationForHost +// for a caller that lists the nodes only when the host names a linked node. +func sharedSystemAlertCorrelationForHostLazy(host models.Host, nodes func() []models.Node) *alerts.AlertCorrelation { + if strings.TrimSpace(host.LinkedNodeID) == "" || strings.TrimSpace(host.ID) == "" { + return nil + } + return sharedSystemAlertCorrelationForHost(host, nodes()) +} + func sharedSystemAlertCorrelationForHost(host models.Host, nodes []models.Node) *alerts.AlertCorrelation { linkedNodeID := strings.TrimSpace(host.LinkedNodeID) hostID := strings.TrimSpace(host.ID) diff --git a/internal/monitoring/monitor_alert_sync.go b/internal/monitoring/monitor_alert_sync.go index a53f30a6e6..eb55d9aa5d 100644 --- a/internal/monitoring/monitor_alert_sync.go +++ b/internal/monitoring/monitor_alert_sync.go @@ -175,8 +175,9 @@ func (m *Monitor) pruneStaleDockerAlerts() bool { // The inventory is the fixture read view in mock mode. A prune that read // it must not remove Docker alerts the other mode raised after a switch. + // Only the hosts' IDs matter, so the view may predate the last metric ticks. scope := m.mockModeFence.begin() - readState := m.currentModeReadState() + readState := m.currentModeStructureReadState() if readState == nil { return false } diff --git a/internal/monitoring/monitor_host_agents_test.go b/internal/monitoring/monitor_host_agents_test.go index 46cf04cea2..a859ed65f4 100644 --- a/internal/monitoring/monitor_host_agents_test.go +++ b/internal/monitoring/monitor_host_agents_test.go @@ -6173,6 +6173,151 @@ func TestMockMetricsTargetRebuildsWhenTheLinkListMoves(t *testing.T) { } } +// The estate view is keyed on the fixture data version, which advances on +// every metric tick, and a build costs about half a second of CPU on an idle +// core (several seconds on a starved process). Callers that read the estate's +// identity and topology only share the view built for the current fixture +// structure instead (currentStructureUnifiedStateView); startup runs every one +// of them before the listener opens, and on a starved process each used to +// build its own estate. +func TestMockStructureConsumersDoNotRebuildTheViewAfterFixtureTicks(t *testing.T) { + useMockEstate(t, 4, time.Second) + m := &Monitor{ + state: models.NewState(), + resourceStore: unifiedresources.NewMonitorAdapter(unifiedresources.NewRegistry(unifiedresources.NewMemoryStore())), + alertManager: alerts.NewManager(), + } + t.Cleanup(m.alertManager.Stop) + + warm := m.currentUnifiedStateView() + waitForFixtureTicks(t, 2) + + consumers := []struct { + name string + run func() + }{ + {"structure read state", func() { m.GetUnifiedStructureReadState() }}, + {"mode structure read state", func() { m.currentModeStructureReadState() }}, + {"structure node listing", func() { m.structureNodes() }}, + {"docker alert prune", func() { m.pruneStaleDockerAlerts() }}, + {"host agent evaluation", func() { m.evaluateHostAgents(time.Now()) }}, + {"chart cache prewarm", func() { m.prewarmMockDashboardChartCaches() }}, + {"host agent evaluation with an offline linked host", func() { + m.state.UpsertHost(models.Host{ID: "offline-agent", Hostname: "offline-agent", LinkedNodeID: "pve1-node", Status: "online", LastSeen: time.Now().Add(-24 * time.Hour)}) + m.evaluateHostAgents(time.Now()) + }}, + } + for _, consumer := range consumers { + consumer.run() + after, ok := publishedMockView(m) + if !ok || after.readState != warm.readState { + t.Fatalf("%s rebuilt the unified view after a metric tick; it only reads identity and topology, which the view built for the current fixture structure holds", consumer.name) + } + } +} + +// The host-agent evaluation lists nodes only for a host that names a linked +// node, and only when it has to. +func TestSharedSystemAlertCorrelationForHostLazyListsNodesOnDemand(t *testing.T) { + listed := 0 + nodes := func() []models.Node { + listed++ + return []models.Node{{ID: "node-1", Instance: "lab", LinkedAgentID: "host-1"}} + } + + if got := sharedSystemAlertCorrelationForHostLazy(models.Host{ID: "host-1"}, nodes); got != nil || listed != 0 { + t.Fatalf("an unlinked host listed nodes %d times and correlated %v", listed, got) + } + if got := sharedSystemAlertCorrelationForHostLazy(models.Host{LinkedNodeID: "node-1"}, nodes); got != nil || listed != 0 { + t.Fatalf("a host without an ID listed nodes %d times and correlated %v", listed, got) + } + got := sharedSystemAlertCorrelationForHostLazy(models.Host{ID: "host-1", LinkedNodeID: "node-1"}, nodes) + if listed != 1 { + t.Fatalf("a linked host listed nodes %d times, want once", listed) + } + want := sharedSystemAlertCorrelationForHost(models.Host{ID: "host-1", LinkedNodeID: "node-1"}, nodes()) + if got == nil || want == nil || got.Key != want.Key { + t.Fatalf("a linked host correlated %+v, want %+v", got, want) + } +} + +// The data-version view must keep serving the callers that read values: a +// consumer of statuses, sensors and metrics must see the tick, so the +// structure view cannot have replaced it. +func TestMockDataVersionViewStillRebuildsAfterFixtureTicks(t *testing.T) { + useMockEstate(t, 3, time.Second) + m := newMockEstateMonitor() + + warm := m.currentUnifiedStateView() + waitForFixtureTicks(t, 1) + + if m.GetUnifiedReadStateOrSnapshot() == warm.readState { + t.Fatal("GetUnifiedReadStateOrSnapshot served the view from before a metric tick") + } +} + +// The structure view answers for the estate it was built from. A structural +// change must reach its callers, not keep serving the old resource set. +func TestMockStructureReadStateFollowsAStructuralFixtureChange(t *testing.T) { + useMockEstate(t, 3, 5*time.Minute) + m := newMockEstateMonitor() + + before := m.GetUnifiedStructureReadState() + if before == nil || len(before.Nodes()) == 0 { + t.Fatal("the mock estate lists no nodes") + } + + grown := mock.GetConfig() + grown.NodeCount++ + mock.SetMockConfig(grown) + + after := m.GetUnifiedStructureReadState() + if after == before { + t.Fatal("the structure view outlived a structural fixture change") + } + if len(after.Nodes()) <= len(before.Nodes()) { + t.Fatalf("the structure view lists %d nodes after the estate grew from %d", len(after.Nodes()), len(before.Nodes())) + } +} + +// An operator link folds one resource into another; the structure view must +// stop listing the folded resource once the resource store applies the link. +func TestMockStructureReadStateFollowsAnOperatorLink(t *testing.T) { + useMockEstate(t, 4, 5*time.Minute) + store := unifiedresources.NewMemoryStore() + m := &Monitor{ + state: models.NewState(), + resourceStore: unifiedresources.NewMonitorAdapter(unifiedresources.NewRegistry(store)), + alertManager: alerts.NewManager(), + } + t.Cleanup(m.alertManager.Stop) + + var vmID, agentID string + for _, resource := range m.currentStructureUnifiedStateView().resources { + switch { + case vmID == "" && resource.Type == unifiedresources.ResourceTypeVM && resource.Agent == nil: + vmID = resource.ID + case agentID == "" && resource.Type == unifiedresources.ResourceTypeAgent && resource.Agent != nil && + len(resource.Sources) == 1 && resource.Sources[0] == unifiedresources.SourceAgent: + agentID = resource.ID + } + } + if vmID == "" || agentID == "" { + t.Fatalf("fixture graph needs an agentless VM and a standalone agent, got vm=%q agent=%q", vmID, agentID) + } + + if err := store.AddLink(unifiedresources.ResourceLink{ResourceA: vmID, ResourceB: agentID, PrimaryID: vmID}); err != nil { + t.Fatalf("AddLink: %v", err) + } + m.updateResourceStore(models.StateSnapshot{}, m.mockModeFence.begin()) + + for _, resource := range m.currentStructureUnifiedStateView().resources { + if resource.ID == agentID { + t.Fatalf("the structure view still lists the agent %q the operator folded into %q", agentID, vmID) + } + } +} + // TestMockModeDiscardsRealHostReports pins the push-side of the mock clean // room. Mock mode already suspends pull-based PVE/PBS/PMG collection, but a // real agent keeps POSTing regardless; ingesting those reports lands a real diff --git a/internal/monitoring/monitor_metrics.go b/internal/monitoring/monitor_metrics.go index e2eb1e9a16..4b0c811f95 100644 --- a/internal/monitoring/monitor_metrics.go +++ b/internal/monitoring/monitor_metrics.go @@ -3,6 +3,7 @@ package monitoring import ( "sort" "strings" + "sync" "time" "github.com/rcourtman/pulse-go-rewrite/internal/mock" @@ -226,7 +227,7 @@ func (m *Monitor) GetStorageMetricsForChart(storageID string, duration time.Dura inMemoryResult = m.metricsHistory.GetAllStorageMetrics(storageID, duration) } if mock.IsMockEnabled() { - return m.mockStorageMetricsForChartCached(storageID, duration, inMemoryResult) + return m.mockStorageMetricsForChartCached(storageID, duration, inMemoryResult, m.GetUnifiedReadStateOrSnapshot) } if m.metricsStore == nil { return inMemoryResult @@ -729,12 +730,14 @@ func (m *Monitor) GetStorageMetricsForChartBatch( result := make(map[string]map[string][]MetricPoint, len(storageIDs)) if mock.IsMockEnabled() { + // One read of the estate view for every pool that needs a total. + readState := sync.OnceValue(m.GetUnifiedReadStateOrSnapshot) for _, sid := range storageIDs { inMemory := map[string][]MetricPoint{} if m.metricsHistory != nil { inMemory = m.metricsHistory.GetAllStorageMetrics(sid, duration) } - result[sid] = m.mockStorageMetricsForChartCached(sid, duration, inMemory) + result[sid] = m.mockStorageMetricsForChartCached(sid, duration, inMemory, readState) } return result } @@ -1179,7 +1182,7 @@ func (m *Monitor) prewarmMockWorkloadChartCaches(duration time.Duration) { } requests := boundMockWorkloadChartPrewarmRequests( - mockWorkloadChartRequestsForReadState(m.GetUnifiedReadStateOrSnapshot()), + mockWorkloadChartRequestsForReadState(m.GetUnifiedStructureReadState()), ) if len(requests.vms) > 0 { _ = m.GetGuestMetricsForChartBatch("vm", requests.vms, duration) @@ -1309,6 +1312,7 @@ func (m *Monitor) mockStorageMetricsForChartCached( storageID string, duration time.Duration, inMemoryResult map[string][]MetricPoint, + readState func() unifiedresources.ReadState, ) map[string][]MetricPoint { cacheKey := mockChartMetricMapCacheKey{ kind: "storage", @@ -1325,7 +1329,7 @@ func (m *Monitor) mockStorageMetricsForChartCached( computed = downsampleMetricMapForMockChart(inMemoryResult, duration) } else { computed = downsampleMetricMapForMockChart( - m.mockStorageMetricsForChart(storageID, duration, inMemoryResult), + m.mockStorageMetricsForChart(storageID, duration, inMemoryResult, readState), duration, ) } diff --git a/internal/monitoring/monitor_metrics_slo_test.go b/internal/monitoring/monitor_metrics_slo_test.go index f59de6ea6e..756177dd3a 100644 --- a/internal/monitoring/monitor_metrics_slo_test.go +++ b/internal/monitoring/monitor_metrics_slo_test.go @@ -823,3 +823,33 @@ func TestMockChartCacheInvalidatesAfterMockHistoryRefresh(t *testing.T) { }, lastPoint) } } + +// A storage chart request totals each pool whose chart is not cached, and a +// linked pool takes its capacity from whichever source was seen last, which +// moves without a structure change, so the totals read the data-version view. +// A batch hands every pool one shared read (GetStorageMetricsForChartBatch): +// the view is rebuilt after a tick, and on a starved process a read per pool +// rebuilt it for each. A pool whose chart is cached reads nothing. +func TestMockStorageChartReadsTheEstateOnlyForAnUncachedChart(t *testing.T) { + useMockEstate(t, 4, 5*time.Minute) + m := newMockEstateMonitor() + pools := m.currentUnifiedStateView().readState.StoragePools() + if len(pools) == 0 { + t.Fatal("the mock estate lists no storage pool") + } + id := pools[0].SourceID() + + reads := 0 + readState := func() unifiedresources.ReadState { + reads++ + return m.GetUnifiedReadStateOrSnapshot() + } + first := m.mockStorageMetricsForChartCached(id, time.Hour, nil, readState) + if reads != 1 || len(first["total"]) == 0 { + t.Fatalf("an uncached chart read the estate %d times and returned %d total points, want one read and a total series", reads, len(first["total"])) + } + m.mockStorageMetricsForChartCached(id, time.Hour, nil, readState) + if reads != 1 { + t.Fatalf("a cached chart read the estate again: %d reads", reads) + } +} diff --git a/internal/monitoring/unified_structure_view.go b/internal/monitoring/unified_structure_view.go new file mode 100644 index 0000000000..00c7fe8d24 --- /dev/null +++ b/internal/monitoring/unified_structure_view.go @@ -0,0 +1,78 @@ +package monitoring + +import ( + "github.com/rcourtman/pulse-go-rewrite/internal/mock" + "github.com/rcourtman/pulse-go-rewrite/internal/models" + "github.com/rcourtman/pulse-go-rewrite/internal/unifiedresources" +) + +// cachedMockStructureView returns the mock view the monitor has cached when it +// was built for the fixture's current structure revision and operator link +// list, however many metric ticks ago. The second result is false when there is +// no such view, and the first is then not for use. +func (m *Monitor) cachedMockStructureView() (monitorUnifiedStateView, bool) { + structure := mock.FixtureStructureRevision() + links := m.resourceStoreManualLinks() + + m.mockUnifiedViewMu.Lock() + defer m.mockUnifiedViewMu.Unlock() + reusable := m.mockUnifiedViewValid && + m.mockUnifiedViewStructure == structure && + sameManualLinks(m.mockUnifiedViewLinks, links) + return m.mockUnifiedView, reusable +} + +// currentStructureUnifiedStateView is currentUnifiedStateView for a caller that +// reads what the estate lists and how it connects: IDs, names, types, source +// mappings, parents, operator links and the metrics targets those yield. In +// mock mode that answers from the cached view for as long as the fixture +// structure and the link list are unchanged. The view is keyed on the data +// version otherwise, which advances on every metric tick, so a caller that +// needs only structure but asked for the data-version view built the whole +// estate once per tick: about half a second of CPU on an idle core, and on a +// starved process a build longer than the tick, so every call missed. A +// caller must not read what a tick moves (status, metric values, timestamps, +// sensors); the view it gets can be any number of ticks old. Outside mock mode +// this is currentUnifiedStateView. +func (m *Monitor) currentStructureUnifiedStateView() monitorUnifiedStateView { + if m == nil { + return monitorUnifiedStateView{} + } + if mock.IsMockEnabled() { + if view, ok := m.cachedMockStructureView(); ok { + return view + } + } + return m.currentUnifiedStateView() +} + +// GetUnifiedStructureReadState is GetUnifiedReadStateOrSnapshot for a caller +// that reads identity and topology only (see currentStructureUnifiedStateView): +// in mock mode it can be several metric ticks old. +func (m *Monitor) GetUnifiedStructureReadState() unifiedresources.ReadState { + if m == nil { + return nil + } + if !mock.IsMockEnabled() { + return m.GetUnifiedReadStateOrSnapshot() + } + return m.currentStructureUnifiedStateView().readState +} + +// currentModeStructureReadState is currentModeReadState for a caller that reads +// identity and topology only. +func (m *Monitor) currentModeStructureReadState() unifiedresources.ReadState { + if m == nil { + return nil + } + if mock.IsMockEnabled() { + return m.currentStructureUnifiedStateView().readState + } + return m.currentModeReadState() +} + +// structureNodes is NodesSnapshot for a caller that reads the nodes' identity +// and links only (ID, instance, linked agent). +func (m *Monitor) structureNodes() []models.Node { + return nodesSnapshotFromReadState(m.GetUnifiedStructureReadState()) +}