mirror of
https://github.com/rcourtman/Pulse.git
synced 2026-09-10 02:25:56 +00:00
fix(hostmetrics): retire disk probes before publishing results
A completed probe remained discoverable after its first caller returned, allowing the next collection to reuse stale filesystem measurements. Remove it and publish completion within one registry critical section. Keep in-flight sharing, cancellation and timeout behaviour intact. The controlled regression fails before this change. Twenty full package runs and three race runs pass on the worker. Refs #1928
This commit is contained in:
rcourtman
@@ -1023,3 +1023,35 @@ and receipt are under `tmp/patrol-storage-docs-proof/` at the workspace root.
|
||||
The served guide SHA-256 is
|
||||
`1970ed5cd70e976d2acbeaf7d36a7b78b5dedf65355c50a19abdba8ca96c9770`.
|
||||
No model or infrastructure action is used by this browser proof.
|
||||
|
||||
## Disk probe completion ordering, 2026-09-06
|
||||
|
||||
Build and Test run `34011979848` on `f26668aa6ddc` reports a failure in
|
||||
`TestCollectDisksExcludesFreeBSDFdescfsBeforeUsage`: the expected root filesystem
|
||||
read did not invoke its usage probe. The shared in-flight registry published a
|
||||
result before removing the completed entry. A subsequent collection could reuse
|
||||
that completed result instead of taking a fresh measurement.
|
||||
|
||||
A controlled regression holds the registry lock while the syscall completes.
|
||||
It fails on the preceding implementation because the caller returns while its
|
||||
completed probe remains discoverable. Retirement and completion publication now
|
||||
share one critical section. The syscall and caller waits remain outside the lock,
|
||||
and overlapping collectors still share genuinely running probes. This corrects
|
||||
the shared ordering contract instead of clearing state or retrying the test.
|
||||
|
||||
On pulse-dev with Go 1.26.8, twenty full hostmetrics package runs pass in
|
||||
16.687s and three complete race-detector runs pass in 1.864s. These include
|
||||
excluded mounts, stalled mounts, recovery, shared results and cancellation.
|
||||
Production source SHA-256 is
|
||||
`cbf5efa6bfcc163faa061ccf7c70bb6738c4ef2f78473d7d66fa00c589f87555`.
|
||||
Regression file SHA-256 is
|
||||
`e7a3adfe7f2cbc36cd8c315ef71ec019fa34db45b75458fa0b2212e3533b09b7`.
|
||||
Both hashes match before and after proof. The raw worker log is
|
||||
`/opt/pulse-release-worker/patrol-disk-probe-completion.log`.
|
||||
|
||||
The preceding storage slice was committed and pushed as `618700db5e` to
|
||||
PR #1928, which remains open. Its exact staged hook passed 163 tests in 127.989s with all fourteen
|
||||
file hashes unchanged. This supersedes the pending-hook statements above for that
|
||||
slice only. Current remote CI is not a completed pass. The f266 benchmark job
|
||||
also failed and its comparison remains under investigation. The overall goal,
|
||||
real-model/action qualification and provider refusal remain open and unchanged.
|
||||
|
||||
@@ -10201,7 +10201,7 @@
|
||||
},
|
||||
{
|
||||
"id": "patrol-assistant-customer-outcome-qualification",
|
||||
"summary": "The explicit Patrol/Assistant redesign goal and ordered source-bound evidence are recorded in docs/qualification/PATROL_ASSISTANT_CUSTOMER_JOURNEY.md. Observations, conclusions, proposals, execution and independently verified outcomes remain distinct, with Assistant continuing the same issue and governed action records. The 2026-09-05 telemetry review found 127 paid installs, 71 with Patrol enabled and 23 with Assistant calls, but fourteen verified resolutions came from one install and schema 17 outcome/provider/cost fields had no adoption. Activity does not establish useful linked tasks. Implemented corrections preserve canonical risk, evidence provenance, retained history, failed-read status and the original diagnostic conversation while removing proposal-as-root-cause promotion, duplicate causal inference, contextless evaluation passes, report quotas and call-count completion proxies. Full affected package and focused race proofs, real ordinary failed-read verification and linked-issue browser matrices are recorded in the plan. Integrated presentation passes 18 delivery-ordering and 12 Overview light/dark cases at 1440/900/390 widths, plus the Patrol/Assistant journey. The qualification schema accepts the implemented health_process_stop injector and all eleven manifests validate. Integration f779bf064ab4 passed all eight Core E2E shards but Build and Test run 34008823529 failed fifteen benchmark comparisons against exact main 3f74c0c27304. An initial performance correction still failed the raw-read comparison and was revised. The final reader reuses bounded SQL templates/current numbered bindings, avoids repeated absent-tier probes within the same snapshot and appends output series directly. Ten alternating worker samples of complete selected query, batch, fleet, API, chart and route benchmark families show no significant greater-than-10% time, byte or allocation regression against that exact base. Final full metrics/database proof passes (77.265s/0.272s), as does focused retained/binding/batch race proof (5.349s). Latency-based concurrent SLO tests pass normally but skip under race. A separate concurrent binding regression with eight scopes and a one-connection pool passes normally (0.043s) and under race (2.106s). Exact staged hook and remote landing remain pending. An earlier native SQLite binding fault has no established root cause. The latest ordinary model diagnosis still makes unsupported temporal claims and remains unqualified despite the successful failed-read contract check. Claude Max explicitly refused autonomous readiness, and cached refusal/API409 enforcement remains intact. Ordinary Assistant success does not qualify autonomy. Required healthy/unhealthy, dependency, missing-access, storage/backup and approved/rejected action outcome qualification remains incomplete. The live catalogue now has a bounded service-storage scenario with independent filesystem measurements. Full qualification/CLI proof passes (4.223s/0.005s), and a provider-free Docker oracle verifies ENOSPC, zero free space, unhealthy service/healthy control, recovery, refused overwrite and symlink writes, and unchanged pre-existing inventory (final live test 8.65s). This qualifies only the fault/oracle, not model diagnosis. Missing-access at the Pulse source/tool boundary and broader host/storage-pool or backup cases remain absent. Alternate paid-provider approval remains pending. No paid request or production mutation was performed. Only run-owned disposable worker containers were faulted and removed. The published schema now accepts equivalent summary-term groups used by the new and three existing action scenarios. All twelve manifests and five full-schema regression tests pass using the CI-pinned validator. The final shipped guide passes Playwright and pixel inspection at /docs/AI_PATROL_QUALIFICATION, 1440/900/390x1000, including catalogue/section/command/limits, horizontal command scrolling, reload and keyboard index/history navigation. Storage slice staged hook and landing remain pending. Volunteered Pro environments remain a separate wider-readiness gate.",
|
||||
"summary": "The explicit Patrol/Assistant redesign goal and ordered source-bound evidence are recorded in docs/qualification/PATROL_ASSISTANT_CUSTOMER_JOURNEY.md. Observations, conclusions, proposals, execution and independently verified outcomes remain distinct, with Assistant continuing the same issue and governed action records. The 2026-09-05 telemetry review found 127 paid installs, 71 with Patrol enabled and 23 with Assistant calls, but fourteen verified resolutions came from one install and schema 17 outcome/provider/cost fields had no adoption. Activity does not establish useful linked tasks. Implemented corrections preserve canonical risk, evidence provenance, retained history, failed-read status and the original diagnostic conversation while removing proposal-as-root-cause promotion, duplicate causal inference, contextless evaluation passes, report quotas and call-count completion proxies. Full affected package and focused race proofs, real ordinary failed-read verification and linked-issue browser matrices are recorded in the plan. Integrated presentation passes 18 delivery-ordering and 12 Overview light/dark cases at 1440/900/390 widths, plus the Patrol/Assistant journey. The qualification schema accepts the implemented health_process_stop injector and all eleven manifests validate. Integration f779bf064ab4 passed all eight Core E2E shards but Build and Test run 34008823529 failed fifteen benchmark comparisons against exact main 3f74c0c27304. An initial performance correction still failed the raw-read comparison and was revised. The final reader reuses bounded SQL templates/current numbered bindings, avoids repeated absent-tier probes within the same snapshot and appends output series directly. Ten alternating worker samples of complete selected query, batch, fleet, API, chart and route benchmark families show no significant greater-than-10% time, byte or allocation regression against that exact base. Final full metrics/database proof passes (77.265s/0.272s), as does focused retained/binding/batch race proof (5.349s). Latency-based concurrent SLO tests pass normally but skip under race. A separate concurrent binding regression with eight scopes and a one-connection pool passes normally (0.043s) and under race (2.106s). Exact staged hook and remote landing remain pending. An earlier native SQLite binding fault has no established root cause. The latest ordinary model diagnosis still makes unsupported temporal claims and remains unqualified despite the successful failed-read contract check. Claude Max explicitly refused autonomous readiness, and cached refusal/API409 enforcement remains intact. Ordinary Assistant success does not qualify autonomy. Required healthy/unhealthy, dependency, missing-access, storage/backup and approved/rejected action outcome qualification remains incomplete. The live catalogue now has a bounded service-storage scenario with independent filesystem measurements. Full qualification/CLI proof passes (4.223s/0.005s), and a provider-free Docker oracle verifies ENOSPC, zero free space, unhealthy service/healthy control, recovery, refused overwrite and symlink writes, and unchanged pre-existing inventory (final live test 8.65s). This qualifies only the fault/oracle, not model diagnosis. Missing-access at the Pulse source/tool boundary and broader host/storage-pool or backup cases remain absent. Alternate paid-provider approval remains pending. No paid request or production mutation was performed. Only run-owned disposable worker containers were faulted and removed. The published schema now accepts equivalent summary-term groups used by the new and three existing action scenarios. All twelve manifests and five full-schema regression tests pass using the CI-pinned validator. The final shipped guide passes Playwright and pixel inspection at /docs/AI_PATROL_QUALIFICATION, 1440/900/390x1000, including catalogue/section/command/limits, horizontal command scrolling, reload and keyboard index/history navigation. Storage slice staged hook and landing remain pending. Volunteered Pro environments remain a separate wider-readiness gate. Storage slice 618700db5e is pushed to open PR #1928 after the exact staged hook passed 163 tests in 127.989s with fourteen unchanged file hashes. CI on f266 exposes a shared disk-probe completion race and another benchmark failure. A controlled regression reproduces result publication before registry retirement. The canonical fix makes retirement and publication atomic to later collectors. Twenty full hostmetrics runs pass (16.687s), as do three complete race runs (1.864s), with source hashes recorded in the plan. Benchmark comparison investigation and final disk-fix staged hook/landing remain pending. These checks do not qualify model diagnosis or action outcomes.",
|
||||
"owner": "project-owner",
|
||||
"status": "planned",
|
||||
"recorded_at": "2026-09-05",
|
||||
|
||||
@@ -2947,6 +2947,14 @@ explicitly mapped administrator reaches the lifecycle handlers.
|
||||
1. Update this contract when agent lifecycle ownership changes. Routes added under the shared `internal/api/` extension point that are clearly outside lifecycle ownership (for example `POST /api/ai/patrol/preflight`, the `patrol_preflight` snapshot field added to `/api/settings/ai`, the auto-trigger preflight dispatch on settings save, the startup-seed dispatch in `NewAISettingsHandler`, and the cached-preflight integration into the Patrol `tools` readiness check — all owned by ai-runtime) do not extend this subsystem's contract; they live in their owning subsystem. Canonical scoped Patrol resolution on `POST /api/ai/patrol/run` and structured `patrol_assess_finding` lifecycle outcomes are likewise adjacent AI/API contracts: they may consume agent-reported identities and evidence, but they do not change agent registration, install, token, profile, command transport, update, or fleet-lifecycle authority.
|
||||
2. Keep shared API proof routing aligned whenever install, register, or profile payloads change.
|
||||
3. Update runtime and settings tests in the same slice when lifecycle behavior changes. Shell installer lifecycle changes must keep `scripts/installtests/install_sh_test.go` covering explicit flags, persisted connection state, legacy running-process/service recovery, legacy single-dash v5 agent flag recovery, repeated disk-exclusion recovery, and secure token-file service argument rendering for update re-entry. Host metrics tests must prove excluded FreeBSD `fdescfs` mounts are filtered before filesystem usage is attempted.
|
||||
Shared host/Docker filesystem probes must retire a completed probe and
|
||||
publish its result under the same registry lock. A later collection must
|
||||
not reuse a completed probe after an earlier caller has received its result.
|
||||
Running probes remain shared across collectors, and cancellation or timeout
|
||||
must not admit a duplicate syscall while the original is still running.
|
||||
`TestGuardedDiskUsagePublishesAfterRetiringProbe` controls the retirement
|
||||
lock to prove publication cannot get ahead of removal. The adjacent stuck
|
||||
mount tests retain sharing, recovery, timeout and cancellation coverage.
|
||||
4. Keep host-agent test hooks, command-client factories, and timing overrides
|
||||
instance-scoped under `internal/hostagent/agent.go`; lifecycle-owned
|
||||
registration and update paths must not depend on package-global mutable test
|
||||
|
||||
@@ -1391,6 +1391,7 @@
|
||||
"internal/dockeragent/agent.go",
|
||||
"internal/dockeragent/container_update.go",
|
||||
"internal/dockeragent/container_update_typed.go",
|
||||
"internal/hostmetrics/collector.go",
|
||||
"internal/kubernetesagent/agent.go",
|
||||
"internal/models/models.go",
|
||||
"internal/monitoring/monitor.go",
|
||||
@@ -1695,6 +1696,20 @@
|
||||
"internal/hostagent/proxmox_setup_test.go"
|
||||
]
|
||||
},
|
||||
{
|
||||
"id": "host-disk-probe-runtime",
|
||||
"label": "host disk probe freshness, sharing, cancellation and timeout proof",
|
||||
"match_prefixes": [],
|
||||
"match_files": [
|
||||
"internal/hostmetrics/collector.go"
|
||||
],
|
||||
"allow_same_subsystem_tests": false,
|
||||
"test_prefixes": [],
|
||||
"exact_files": [
|
||||
"internal/hostmetrics/collector_stuck_mount_test.go",
|
||||
"internal/hostmetrics/collector_test.go"
|
||||
]
|
||||
},
|
||||
{
|
||||
"id": "unified-agent-runtime",
|
||||
"label": "unified agent runtime proof",
|
||||
|
||||
@@ -117,10 +117,11 @@ func guardedDiskUsage(ctx context.Context, mountpoint string) (*godisk.UsageStat
|
||||
if !loaded {
|
||||
go func() {
|
||||
call.usage, call.err = diskUsage(ctx, mountpoint)
|
||||
// Publish the result before admitting a new probe.
|
||||
close(call.done)
|
||||
// Retire and publish together so a caller returning this result
|
||||
// cannot leave a completed probe available to a later collection.
|
||||
stuckDiskMounts.Lock()
|
||||
delete(stuckDiskMounts.calls, mountpoint)
|
||||
close(call.done)
|
||||
stuckDiskMounts.Unlock()
|
||||
if time.Now().After(call.deadline) {
|
||||
log.Info().Str("mount", mountpoint).Msg("disk: stalled usage call returned, mount re-included")
|
||||
|
||||
@@ -318,3 +318,39 @@ func TestGuardedDiskUsage_AlreadyCancelledDoesNotStartProbe(t *testing.T) {
|
||||
t.Fatalf("cancelled collection started %d probes, want none", got)
|
||||
}
|
||||
}
|
||||
|
||||
func TestGuardedDiskUsagePublishesAfterRetiringProbe(t *testing.T) {
|
||||
original := diskUsage
|
||||
defer func() { diskUsage = original }()
|
||||
entered := make(chan struct{})
|
||||
release := make(chan struct{})
|
||||
returned := make(chan error, 1)
|
||||
const mount = "/test/retire-before-publish"
|
||||
diskUsage = func(_ context.Context, path string) (*godisk.UsageStat, error) {
|
||||
close(entered)
|
||||
<-release
|
||||
return &godisk.UsageStat{Path: path, Total: 100}, nil
|
||||
}
|
||||
go func() { _, err := guardedDiskUsage(context.Background(), mount); returned <- err }()
|
||||
<-entered
|
||||
// Hold registry retirement until the first caller has a chance to return.
|
||||
// Publishing before retirement lets the next read reuse a completed probe.
|
||||
stuckDiskMounts.Lock()
|
||||
close(release)
|
||||
premature := false
|
||||
select {
|
||||
case <-returned:
|
||||
premature = true
|
||||
case <-time.After(50 * time.Millisecond):
|
||||
}
|
||||
stuckDiskMounts.Unlock()
|
||||
if premature {
|
||||
t.Fatal("read returned while completed probe was still available to new callers")
|
||||
}
|
||||
if err := <-returned; err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if _, ok := loadDiskUsageCall(mount); ok {
|
||||
t.Fatal("completed probe remains in flight registry")
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user