Skip to content

Fix #5061: provider-aware databaseFilter overload for AddMartenHighWaterHealthCheck - #5089

Merged
jeremydmiller merged 1 commit into
masterfrom
fix/5061-provider-aware-database-filter
Jul 30, 2026
Merged

Fix #5061: provider-aware databaseFilter overload for AddMartenHighWaterHealthCheck#5089
jeremydmiller merged 1 commit into
masterfrom
fix/5061-provider-aware-database-filter

Conversation

@jeremydmiller

Copy link
Copy Markdown
Member

Closes #5061.

The problem

AddMartenHighWaterHealthCheck's databaseFilter is a plain Func<IMartenDatabase, bool> captured at registration time and stored on a HighWaterHealthCheckSettings instance. The closure is created before the container exists, so it can only read what the application already has in hand — an ambient collection, a config value, a static. It cannot resolve services.

That is a problem for the exact topology #4991 added the filter for. Under Wolverine-managed daemon distribution (DaemonMode.ExternallyManaged), which databases a node owns is decided by Wolverine's agent assignments and moves over the node's lifetime — rollouts, node loss, leadership changes. The authoritative source, IWolverineRuntime.Agents.AllLocallyOwnedDatabaseIds(), has to be read per probe. Applications were left either mirroring Wolverine's assignment state into a static, or Replace()-ing Marten's settings registration by hand — duplicating registration semantics that break silently when the record changes.

HighWaterHealthCheck already receives an IServiceProvider; only the filter couldn't reach it.

The approach

Issue suggestion 1 — an overload taking Func<IServiceProvider, IMartenDatabase, bool>, invoked once per database per probe.

The one design constraint worth calling out: the existing method is all-optional parameters, so a second all-optional overload differing only in the filter's delegate type would make the no-argument call ambiguous. Putting the provider-aware filter first, as a required argument keeps both overloads unambiguous and leaves every existing call site untouched, while preserving single-call ergonomics:

Services.AddHealthChecks().AddMartenHighWaterHealthCheck(
    (services, database) => services.GetRequiredService<IWolverineRuntime>()
        .Agents.AllLocallyOwnedDatabaseIds()
        .Any(id => id.Name.EqualsIgnoreCase(database.Identifier)),
    staleThreshold: TimeSpan.FromSeconds(30),
    includeExternallyManaged: true);

It lands on the settings record as ScopedDatabaseFilter and is applied in CheckHealthAsync against the provider the check itself was resolved from — so scoped services are legal too. When both filters are set, a database must satisfy both.

Also picks up suggestion 3 at no extra cost: both overloads now register the settings through a Singleton factory rather than a bare instance, so replacing HighWaterHealthCheckSettings remains a supported extension point for anything the predicate can't express. That is what the reporter's workaround was reaching for; it now works against a documented shape.

Suggestion 2 (an IHighWaterDatabaseFilter service) was skipped deliberately — it adds a second registration surface for the same decision, and the factory registration already covers the "I need to reach further into DI" case.

Tests

Three added to HighWaterHealthCheckTests next to the existing #4991 filter tests, using a fake ownership registry resolved from DI as a stand-in for IWolverineRuntime:

  • scoped_database_filter_excluding_the_database_reports_healthy_despite_stuck_mark — the non-owned database is never probed, even past the staleness threshold.
  • scoped_database_filter_is_re_evaluated_on_every_probe — the headline behavior: a node that doesn't own the database at probe 1 but does at probe 2 starts asserting on it with no re-registration.
  • scoped_overload_registers_the_settings_through_a_factory_carrying_the_filter — registration shape.

All 22 tests in the file pass on net9.0.

Docs: a new "When ownership is runtime state" subsection under the sharded/multi-tenant guidance in docs/events/projections/healthchecks.md, with the databaseFilter parameter doc now pointing at the new overload. markdownlint + cspell clean.

🤖 Generated with Claude Code

…Check

The existing `databaseFilter` is a plain `Func<IMartenDatabase, bool>`
captured at registration time and stored on a settings instance, so it can
only read what the application already has in hand. It cannot resolve
services -- which makes it unable to express "the databases this node
currently owns" when ownership is runtime state.

That is exactly the Wolverine-managed distribution case #4991 added the
filter for: Wolverine assigns projection/subscription agents per
(database, tenant) and rebalances them over a node's lifetime, so the
authoritative answer (`IWolverineRuntime.Agents.AllLocallyOwnedDatabase
Ids()`) has to be read per probe. Applications were left mirroring
Wolverine's assignment state into a static, or replacing Marten's settings
registration by hand -- duplicating registration semantics that break
silently when the record changes.

Adds an overload taking `Func<IServiceProvider, IMartenDatabase, bool>`
as its leading argument (issue suggestion 1). A required delegate-typed
first parameter keeps the two all-optional overloads unambiguous, so
existing calls are untouched:

    Services.AddHealthChecks().AddMartenHighWaterHealthCheck(
        (services, database) => services.GetRequiredService<IWolverineRuntime>()
            .Agents.AllLocallyOwnedDatabaseIds()
            .Any(id => id.Name.EqualsIgnoreCase(database.Identifier)),
        staleThreshold: TimeSpan.FromSeconds(30),
        includeExternallyManaged: true);

It lands on the settings record as `ScopedDatabaseFilter` and is applied
in CheckHealthAsync against the provider the check itself was resolved
from -- so scoped services are legal, and it is evaluated once per
database per probe. When both filters are set a database must satisfy
both.

Both overloads now register the settings through a Singleton *factory*
rather than a bare instance (issue suggestion 3), so replacing them stays
a supported extension point for anything the predicate can't express.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

1 participant