Joseph Doherty
0c2e852a62
(a) Inbound API compile state (#75) is per central node: a recompile cleared the known-bad record only on the node that answered, and the peer kept refusing the unchanged script with SCRIPT_COMPILE_FAILED (the per-request revision check only notices a changed script). Create / update / delete (ManagementActor post-commit), RecompileApiMethod, and API methods written by a Transport bundle import now publish InboundApiMethodInvalidated on the central cache-invalidation topic (#100 item 3). The new InboundApiCacheInvalidationHandler drops that method's compiled handler, known-bad record and cached ApiMethod row on the peer, which compiles the current text on its next request. (b) docs/components/InboundAPI.md no longer claims a direct DB edit stays stale until restart: the per-request revision check picks it up once the node's cached row expires (ApiMethodCacheTtl, 5 s). Component-InboundAPI, Component-Host and the #85 cache audit describe the peer invalidation. (c) Decision: the import's locked-override warnings keep EXACT (ordinal) name matching, because FlatteningService matches override names ordinally. An override differing from a locked member only by case never reaches that member's lock check (the flattener drops it as unresolved), so a "targets a LOCKED member" warning for it would be false. The comparer is now named (LockedOverrideNameComparer) and documented, and pinned by tests on both sides (flattener + import). Tests: the peer's known-bad record is cleared by a recompile (and validate leaves it alone); create/update/delete and the import publish the invalidation; the handler drops only the named method; composition root registers the handler; case-only override gets no lock warning; the flattener matches override names exactly. Claude-Session: https://claude.ai/code/session_01GT67AEHd2Acv1YPwGfXEoV
93 lines
9.0 KiB
Markdown
93 lines
9.0 KiB
Markdown
# Central per-node cache staleness audit (Gitea #85)
|
||
|
||
**Date:** 2026-11-09. **Scope:** every in-process cache on a **central** node that holds
|
||
configuration-database (or secret-store) state, and how stale it can be on the **peer** central
|
||
node after a change is made through the other one. Same defect class as #83
|
||
(`ExternalDefinitionLookupCache`).
|
||
|
||
## Context that sets the risk
|
||
|
||
- Traefik routes UI, `/management` and `/api/*` to the **active** central node only
|
||
(`/health/active`). An edit and a use on different nodes therefore only happen when someone
|
||
talks to a node directly (CLI pointed at a node's own port) or across a **failover**.
|
||
- Every TTL below is measured from when the entry was loaded, so an entry a node cached while it
|
||
was previously active is still bounded by its TTL after it becomes active again.
|
||
- No cache below replicates its invalidation to the peer. Central already has one cross-node
|
||
channel: the `DistributedPubSub` topic `CentralCommunicationActor.HealthReportTopic` (peer
|
||
health replication). A config-invalidation broadcast, if an owner wants one, should be a second
|
||
topic on the same mediator, published from `AfterConfigurationCommit`.
|
||
|
||
## Inventory
|
||
|
||
| Cache | Holds | Local invalidation | Peer staleness bound | Verdict |
|
||
|---|---|---|---|---|
|
||
| `InboundScriptExecutor` compiled handlers (InboundAPI) | Compiled method delegates | `InvalidateMethod` on delete; change bus on import | **None** — every request revision-checks the cached handler against the resolved `ApiMethod.Script` and recompiles on mismatch | OK |
|
||
| `ApiMethodCache` (InboundAPI) | `ApiMethod` rows, incl. not-found | Change bus (bundle import); **management create/update/delete — added by #85** | Change through the other node: **none** (`InboundApiMethodInvalidated`, #103); direct SQL: `InboundApi:ApiMethodCacheTtl`, default **5 s** | OK (local gap fixed by #85, peer by #103) |
|
||
| `ExternalDefinitionLookupCache` (ESG, central only) | External systems / methods / DB connections (found only) | 9 ManagementActor handlers (#65); bundle import (#83) | Edit or import through the other node: **none** (`ExternalDefinitionsInvalidated` on `central-cache-invalidation`, #100); direct SQL: `ExternalSystemGateway:DefinitionCacheTtl`, **10 s** on central | Fixed — #100 item 3 (see D3) |
|
||
| `RoleMappingCache` (Security) — the cache at `ManagementActor` `InvalidateRoleMappingCacheAfterCommit` | LDAP group → role map + Deployer site scopes | Mapping / scope-rule commands, post-commit | **None** (`RoleMappingsInvalidated` on `central-cache-invalidation`, #100); TTL `Security:RoleMappingCacheSeconds` (**30 s**) only if the broadcast is lost | Fixed — #100 item 3 (see D1) |
|
||
| `PendingUpdatesCache` (DeploymentManager, #91) | Fleet pending-updates snapshot | Deployment finished on this node | `DeploymentManager:PendingUpdatesCacheTtl`, default **60 s** | OK — display only |
|
||
| `CentralCommunicationActor` site-address cache (+ `GrpcSiteTransport` channel pairs) | Site gRPC endpoints | Site create/update/delete → `RefreshSiteAddresses` | Periodic DB reload, **60 s** (hard-coded) | OK — rare edit, self-heals |
|
||
| `SiteStreamGrpcClientFactory` clients | Per (site, endpoint) stream clients | Site delete → `RemoveSiteAsync` (local only) | Not stale data: a removed site's client on the peer is reclaimed by idle eviction, `Communication:GrpcClientIdleTtl` (**30 min**) × 1.5 after last use | OK — resource, not correctness |
|
||
| `SitePskProvider` (Host) | Per-site gRPC PSK from config / secret store | Site removal; `SB-GRPC-PSK-{site}` write via `/admin/secrets`; site auth rejection; `SitePsks` reload (#96) | Write via `/admin/secrets`: **none** (peer told over `DistributedPubSub` topic `central-cache-invalidation`, formerly `site-psk-invalidation`); other writes: first rejected call after `Secrets:ResolveCacheTtl` | Fixed — #96 (see D2) |
|
||
| `DefaultSecretResolver` (shared `ZB.MOM.WW.Secrets`) | Decrypted secret values | Lib's own invalidator (UI writes, sync service) | `Secrets:ResolveCacheTtl`, **30 s** | OK — owned by the shared lib |
|
||
| `KpiSnapshotCache` (CentralUI) | KPI query results (operational data) | — | **8 s** | OK — not config |
|
||
| `SharedAlarmSummaryService` (CentralUI) | Per-site alarm summary fan-out | — | **12 s** cold-cache window | OK — not config |
|
||
| `IDeploymentStatusNotifier` (DeploymentManager) | Push of deployment status to UI circuits | In-process event | Peer UI never sees the push, but the UI is only served by the active node and the pages also poll | OK |
|
||
| `ITemplateGraphWatermark` / `FlattenSession` | Template-chain memo keyed by in-process version | Repository bumps | Session lives for one deploy/validate operation; the watermark is process-local by design (peer starts from 0 → extra work, never stale work) | OK |
|
||
| `ScriptCompileVerdictCache`, `TrustVerdictCache`, `AuditRegexCache`, `ScriptAnalysisService` `IMemoryCache`, `OAuth2TokenService` | Pure functions of their key (code hash + surface, pattern, credential hash) | — | Not applicable — a changed input is a different key | OK |
|
||
| `ZB.MOM.WW.Auth.ApiKeys.SqlServer` (inbound API keys) | — | — | No in-process cache in the shared store (checked in the 0.3.0 source) — revocation is immediate on both nodes | OK |
|
||
|
||
## Fixed in #85
|
||
|
||
**`ApiMethodCache` was not invalidated by the management API even locally.** Only the compiled
|
||
handler was dropped, so for up to the 5 s TTL on the node that made the change:
|
||
|
||
- a just-**deleted** method stayed callable — the executor recompiled it from the cached row;
|
||
- a just-**created** method answered 404 if a not-found had been cached.
|
||
|
||
`ManagementActor` now drops the method's cache entry post-commit on create, update and delete
|
||
(`InvalidateApiMethodCacheAfterCommit`), the same post-commit pattern as the role-mapping cache.
|
||
The peer is still bounded by the 5 s TTL. Tests: `ApiMethodCacheInvalidationTests` (3 of 4 fail
|
||
without the fix; the fourth is the negative control).
|
||
|
||
No peer-staleness case was **clearly** a bug: every config cache with peer staleness is
|
||
TTL-bounded (5–60 s), its bound is documented at the cache, and the active-only routing means the
|
||
window is only hit via a direct-to-node call or a failover inside the TTL. So no cross-node
|
||
broadcast was added.
|
||
|
||
## Needs an owner decision
|
||
|
||
- **D1 — `RoleMappingCache` revocation lag on the peer (30 s).** Removing a mapping or narrowing a
|
||
Deployer's site scope takes effect at once on the node that made the change and within 30 s on
|
||
the other. It is a security-relevant window (revoked access keeps working briefly), but only on
|
||
the node the change did *not* go through. Options: accept and document (current state); lower
|
||
`RoleMappingCacheSeconds`; or add the `DistributedPubSub` invalidation topic described above.
|
||
Recommendation: accept unless the operators' revocation SLA is tighter than 30 s.
|
||
- **D2 — `SitePskProvider` caches a successful PSK resolve forever.** Its own remarks say
|
||
`Invalidate` covers "a key rotated in place", but nothing calls it on rotation — only on site
|
||
removal. Rotating `SB-GRPC-PSK-{site}` in the secret store (with the site pair restarted on the
|
||
new key) leaves **both** central nodes dialling with the old key until they restart. Options: make
|
||
central restart part of the rotation runbook; give the cache a TTL; or invalidate on a
|
||
`PermissionDenied` from the site. Not peer-specific, so it is out of #85's fix scope — file it
|
||
separately. **Resolved by #96:** all three — invalidate on the rotation write (and the peer, via
|
||
the new `site-psk-invalidation` topic; the first config-invalidation broadcast topic D1/D3 could
|
||
join), refresh on a site's auth rejection, and drop the cache on a `SitePsks` reload.
|
||
- **D3 — `ExternalDefinitionLookupCache` peer bound (10 s).** Already tracked by #83; the
|
||
recommendation there (accept and document) still stands. If D1 gets a broadcast topic, this cache
|
||
and `ApiMethodCache` should join it rather than add a second mechanism.
|
||
|
||
## Follow-up: one central cache-invalidation topic (#100 item 3, 2026-11-09)
|
||
|
||
D1 and D3 now ride a single `DistributedPubSub` topic, `central-cache-invalidation`, with typed
|
||
messages (`ICentralCacheInvalidation`: `SitePskInvalidated`, `RoleMappingsInvalidated`,
|
||
`ExternalDefinitionsInvalidated`, `InboundApiMethodInvalidated`) instead of one topic per cache.
|
||
The #96 topic was migrated onto it: `SitePskInvalidated` travels on the shared topic, and for one
|
||
release is also published bare on `site-psk-invalidation` (still subscribed), so a mixed-version
|
||
central pair keeps working. Each envelope carries the publisher's origin id and the subscriber
|
||
skips its own. Publishers: ManagementActor post-commit (role mappings, external definitions,
|
||
API methods), the Transport bundle importer (external definitions, API methods), the API-method
|
||
recompile (#103: a recompile now clears a stale known-bad record on the peer too) and the PSK
|
||
invalidator. Every cache keeps its TTL / revision check as the fallback for a
|
||
lost broadcast or an out-of-band (SQL) edit. Design: Component-Host.md → *Central cache
|
||
invalidation*; tests include a real two-node cluster (`CentralCacheInvalidationTwoNodeTests`).
|