fix(central): audit per-node central caches; management API drops ApiMethodCache entries (ScadaBridge#85)
Audit (docs/plans/2026-11-09-central-cache-staleness-audit.md) of every central in-process cache holding configuration or secret state, with its staleness bound on the peer central node. Every config cache with peer staleness is TTL-bounded (5-60 s) and Traefik routes to the active node only, so no peer case was clearly a bug and no cross-node broadcast is added; the doc names the existing DistributedPubSub mediator (health replication topic) as the mechanism if one is wanted. Owner decisions listed: RoleMappingCache 30 s revocation lag on the peer, SitePskProvider caching a PSK forever across an in-place rotation (both nodes), and the #83 ExternalDefinitionLookupCache 10 s bound. Fixed: ManagementActor API-method create/update/delete invalidated only the compiled handler, not ApiMethodCache, so the node that made the change kept serving the pre-change row for up to the 5 s TTL (a deleted method stayed callable; a cached not-found hid a new one). The handlers now drop the entry post-commit, like the role-mapping cache. Tests: ApiMethodCacheInvalidationTests (create/update/delete fail without the fix; a negative control checks other methods stay cached). (ScadaBridge#85) Claude-Session: https://claude.ai/code/session_01GT67AEHd2Acv1YPwGfXEoV
This commit is contained in:
@@ -14,13 +14,15 @@ namespace ZB.MOM.WW.ScadaBridge.InboundAPI;
|
||||
/// <list type="bullet">
|
||||
/// <item><description><b>Fast path.</b> <see cref="ScriptArtifactChangeSubscriber"/> calls
|
||||
/// <see cref="Invalidate"/> for every name in an <c>ApiMethod</c>
|
||||
/// <c>ScriptArtifactsChanged</c> notification (bundle-import changes today).</description></item>
|
||||
/// <c>ScriptArtifactsChanged</c> notification (bundle-import changes), and the
|
||||
/// <c>ManagementActor</c> API-method create/update/delete handlers drop the changed name
|
||||
/// post-commit (Gitea #85). Both are node-local.</description></item>
|
||||
/// <item><description><b>Fallback.</b> Every entry — including a negative (not-found) result —
|
||||
/// expires after <see cref="InboundApiOptions.ApiMethodCacheTtl"/> regardless of whether an
|
||||
/// invalidation notification ever arrives. This is what keeps an operator edit via the
|
||||
/// Management API/UI (which does not publish to the change bus) correct within a bounded
|
||||
/// window, and is also the correctness backstop for the bus's at-least-once/in-process-only
|
||||
/// contract — a missed or duplicate notification is harmless.</description></item>
|
||||
/// invalidation notification ever arrives. This is what bounds staleness on the PEER central
|
||||
/// node (no invalidation crosses nodes), and is also the correctness backstop for the bus's
|
||||
/// at-least-once/in-process-only contract — a missed or duplicate notification is
|
||||
/// harmless.</description></item>
|
||||
/// </list>
|
||||
/// <para>
|
||||
/// A negative (method not found) result is cached too — bounded by the same short TTL — so a
|
||||
|
||||
@@ -41,9 +41,9 @@ public class InboundApiOptions
|
||||
/// before the next request re-fetches it from the repository. The
|
||||
/// <c>ScriptArtifactChangeSubscriber</c>/<c>IScriptArtifactChangeBus</c> pipeline
|
||||
/// invalidates a specific method's entry immediately on a bundle-import change (the fast
|
||||
/// path); this TTL is the self-healing fallback for changes the bus does not cover (e.g.
|
||||
/// an operator edit via the Management API/UI, which does not publish a bus notification),
|
||||
/// bounding staleness without requiring every mutation path to remember to invalidate.
|
||||
/// path), and a Management API/UI create/update/delete drops it post-commit on the node that
|
||||
/// made the change (Gitea #85). Both are node-local, so this TTL is the bound on the PEER
|
||||
/// central node (and the self-healing fallback for any path that forgets to invalidate).
|
||||
/// Short by design — the whole point is to absorb repeated-request-storm SQL, not to
|
||||
/// serve minutes-old method definitions.
|
||||
/// </summary>
|
||||
|
||||
@@ -3811,6 +3811,7 @@ public class ManagementActor : ReceiveActor
|
||||
};
|
||||
await repo.AddApiMethodAsync(method);
|
||||
await repo.SaveChangesAsync();
|
||||
await InvalidateApiMethodCacheAfterCommit(sp, method.Name);
|
||||
// The script is persisted regardless; surface a non-fatal warning when it does
|
||||
// not compile so the caller is not misled into thinking a broken method is live
|
||||
// (a never-compiled method returns "Script compilation failed" on first call).
|
||||
@@ -3832,6 +3833,7 @@ public class ManagementActor : ReceiveActor
|
||||
method.TraceAllCalls = traceAllCalls;
|
||||
await repo.UpdateApiMethodAsync(method);
|
||||
await repo.SaveChangesAsync();
|
||||
await InvalidateApiMethodCacheAfterCommit(sp, method.Name);
|
||||
// The persisted DB row is authoritative: on a compile failure the method returns
|
||||
// a compilation error on every request (ExecuteAsync revision-checks the cached
|
||||
// handler against the fresh row), so the saved-but-broken script does NOT keep a
|
||||
@@ -3849,11 +3851,32 @@ public class ManagementActor : ReceiveActor
|
||||
await repo.DeleteApiMethodAsync(cmd.ApiMethodId);
|
||||
await repo.SaveChangesAsync();
|
||||
if (method != null)
|
||||
{
|
||||
await AfterConfigurationCommit(sp, () => { sp.GetService<InboundAPI.InboundScriptExecutor>()?.InvalidateMethod(method.Name); return Task.CompletedTask; });
|
||||
await InvalidateApiMethodCacheAfterCommit(sp, method.Name);
|
||||
}
|
||||
await AuditAsync(sp, user, "Delete", "ApiMethod", cmd.ApiMethodId.ToString(), method?.Name ?? cmd.ApiMethodId.ToString(), null);
|
||||
return true;
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Drops this node's <see cref="InboundAPI.ApiMethodCache"/> entry for <paramref name="name"/>
|
||||
/// once an API-method create/update/delete has committed (Gitea #85). Without it the
|
||||
/// node that made the change kept serving the pre-change row for up to
|
||||
/// <c>ApiMethodCacheTtl</c>: a just-deleted method stayed callable (the executor recompiled
|
||||
/// it from the cached row), and a cached not-found answered 404 for a just-created one.
|
||||
/// Queued post-commit, like the role-mapping invalidation, so a concurrent request cannot
|
||||
/// re-cache the pre-change row. The peer central node still converges within the TTL
|
||||
/// (default 5 s) — see <c>docs/plans/2026-11-09-central-cache-staleness-audit.md</c>.
|
||||
/// </summary>
|
||||
private static Task InvalidateApiMethodCacheAfterCommit(IServiceProvider sp, string name)
|
||||
{
|
||||
var cache = sp.GetService<InboundAPI.ApiMethodCache>();
|
||||
return cache is null
|
||||
? Task.CompletedTask
|
||||
: AfterConfigurationCommit(sp, () => { cache.Invalidate(name); return Task.CompletedTask; });
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Recompiles and re-registers an inbound API method's script after a Create/Update,
|
||||
/// returning a non-null human-readable warning when the script did NOT compile. The
|
||||
|
||||
Reference in New Issue
Block a user