Skip to content

agent/local: serialize full and partial state syncs to fix stale TTL checks - #24069

Open
akasakariko wants to merge 2 commits into
hashicorp:mainfrom
akasakariko:fix/ttl-check-sync-24064
Open

akasakariko wants to merge 2 commits into
hashicorp:mainfrom
akasakariko:fix/ttl-check-sync-24064

Conversation

@akasakariko

Copy link
Copy Markdown

Description

Fixes #24064

When a full anti-entropy sync (updateSyncState) overlaps a partial sync (SyncChanges), a catalog check can remain critical until a later full sync while local state is passing:

  • Full sync captures a passing Health.NodeChecks snapshot before acquiring the state lock for reconciliation
  • A TTL expiry marks the local check critical (TTL expired) and a concurrent partial SyncChanges pushes critical to the catalog and marks it InSync
  • A heartbeat flips the local check back to passing (dirty)
  • Full sync reconciles against its stale passing Health.NodeChecks snapshot and marks the local passing check InSync, so the catalog check remains critical until a later full sync
  • Repeated identical passing heartbeats do not dirty the check, so partial syncs do not repair the mismatch

This 11-line production change adds syncLock sync.Mutex to State and acquires it with defer release in updateSyncState and SyncChanges before the state lock, serializing full-sync reads and reconciliation against partial writes while letting local UpdateCheck calls proceed during catalog reads; there are no public API, dependency, or configuration changes

Testing & Reproduction steps

TestState_SyncFull_ConcurrentCheckUpdate uses an optional fakeRPC handler to hold Health.NodeChecks during a full sync and reproduce the interleaving above

All validation ran locally on Go 1.26.9 (darwin/arm64):

  • Before fix: go test ./agent/local -run '^TestState_SyncFull_ConcurrentCheckUpdate$' -count=10 — failed 10/10
  • After fix (100/100, 100/100 -race, and affected package suites all PASS):
go test ./agent/local -run '^TestState_SyncFull_ConcurrentCheckUpdate$' -count=100
go test -race ./agent/local -run '^TestState_SyncFull_ConcurrentCheckUpdate$' -count=100 -timeout=5m
go test ./agent/local ./agent/ae ./agent/checks -count=1 -timeout=10m
go test -race ./agent/local ./agent/ae ./agent/checks -count=1 -timeout=10m

Additional validation:

  • Targeted normal ./agent tests for updateTTLCheck, UpdateCheck, UpdateCheck_ACLDeny, and AddServices_AliasUpdateCheckNotReverted passed (without -race)
  • go vet, golangci-lint 2.11.4 (0 issues), gofmt, git diff --check, native binary build, and linux/amd64 affected-package cross-build passed
  • Original issue pause/resume script smoke test on the fixed binary (busy mode, 2 services, 3 cycles with 30s pauses) showed 0 stuck checks
  • One initial package run hit existing TestAgentAntiEntropy_Check_DeferSync timing assertion while -race compilation ran concurrently (root cause unconfirmed); 10 isolated runs on fixed code and 10 on baseline passed, and subsequent full affected suites passed normally and with -race
  • Immediately before PR preparation, 10 focused regression runs, all three affected normal suites, and lint reran and passed
  • Scope limits: full repo suite, full ./agent suite, and GitHub CI were not run

Links

PR Checklist

  • updated test coverage
  • external facing docs updated (not needed; no user-facing or config changes)
  • appropriate backport labels added (left to maintainers)
  • not a security concern

PCI review checklist

  • I have documented a clear reason for, and description of, the change I am making (reason and description in Description)
  • If applicable, I've documented a plan to revert these changes if they require more than reverting the pull request (standard revert suffices)
  • If applicable, I've documented the impact of any changes to security controls (no security-control changes)

@akasakariko
akasakariko requested review from a team as code owners October 21, 2026 12:09
@hashicorp-cla-app

hashicorp-cla-app Bot commented Oct 21, 2026 •

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

@hashicorp-cla-app

Copy link
Copy Markdown

CLA assistant check

Thank you for your submission! We require that all contributors sign our Contributor License Agreement ("CLA") before we can accept the contribution. Read and sign the agreement

Learn more about why HashiCorp requires a CLA and what the CLA includes

Have you signed the CLA already but the status is still pending? Recheck it.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

After a client agent stalls, a TTL check can stay critical in the catalog until the next full sync, although the agent reports it passing

1 participant