From 65d8cbfe2756821ef0d26688310db198f3ec1234 Mon Sep 17 00:00:00 2001 From: "Jeremy D. Miller" Date: Tue, 4 Aug 2026 09:45:20 -0500 Subject: [PATCH] Drop the never-written warning/critical_behind_threshold columns (#5173) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `warning_behind_threshold` and `critical_behind_threshold` were created in DDL, listed in every extended-tracking SELECT, and hydrated into `ShardState` — and written by nothing, in any repo. Every read of them returned NULL, always. Nothing ever owned the value. In JasperFx the two `ShardState` properties are declared and never assigned or read; in CritterWatch the only threshold code path is a stub that logs and discards, and every lag threshold lives console-side. So this drops them rather than wiring them: two columns of storage, two entries in every extended-tracking SELECT and two selector ordinals, all carrying nothing. Existing deployments get an `alter table ... drop column` on their next `ApplyAllConfiguredChangesToDatabaseAsync()`, which is lossless because the columns were provably always NULL. The `failure_*` columns deliberately trail the extended block so `ShardStateSelector`'s positional ordinals stay stable; removing two columns from the middle of that block shifts them, so the existing #5048 round-trip tests are the guard and stay green. The two `ShardState` properties remain in JasperFx (unassigned and unread there too) — a hand-off, not something Marten can remove. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_017CTtw2kVRSZKp1p5RTxgAy --- docs/events/projections/async-daemon.md | 5 +- ...g_5173_behind_threshold_columns_removed.cs | 63 +++++++++++++++++++ .../Progress/ProjectionProgressStatement.cs | 2 +- .../Daemon/Progress/ShardStateSelector.cs | 14 +---- .../Events/Schema/EventProgressionTable.cs | 10 ++- 5 files changed, 77 insertions(+), 17 deletions(-) create mode 100644 src/DaemonTests/Bugs/Bug_5173_behind_threshold_columns_removed.cs diff --git a/docs/events/projections/async-daemon.md b/docs/events/projections/async-daemon.md index fd6fbd8487..6f6fe80a91 100644 --- a/docs/events/projections/async-daemon.md +++ b/docs/events/projections/async-daemon.md @@ -642,9 +642,8 @@ that just emits and update every time that Marten has to "skip" stale events. ## Extended Progression Tracking -Extended progression tracking adds ten monitoring columns (`heartbeat`, -`agent_status`, `pause_reason`, `running_on_node`, `warning_behind_threshold`, -`critical_behind_threshold`, `failure_category`, `failure_event_sequence`, +Extended progression tracking adds eight monitoring columns (`heartbeat`, +`agent_status`, `pause_reason`, `running_on_node`, `failure_category`, `failure_event_sequence`, `failure_event_type`, `failure_event_tenant_id`) to `mt_event_progression`. The async daemon writes them from existing runtime state and the shard-state selector reads them back into `ShardState` so monitoring tooling such as CritterWatch can display diff --git a/src/DaemonTests/Bugs/Bug_5173_behind_threshold_columns_removed.cs b/src/DaemonTests/Bugs/Bug_5173_behind_threshold_columns_removed.cs new file mode 100644 index 0000000000..3d01fba235 --- /dev/null +++ b/src/DaemonTests/Bugs/Bug_5173_behind_threshold_columns_removed.cs @@ -0,0 +1,63 @@ +using System.Collections.Generic; +using System.Threading.Tasks; +using JasperFx.Events; +using Marten.Testing; +using Marten.Testing.Harness; +using Npgsql; +using Shouldly; +using Weasel.Postgresql; +using Xunit; + +namespace DaemonTests.Bugs; + +/// +/// #5173 — warning_behind_threshold and critical_behind_threshold were created in DDL, +/// listed in every extended-tracking SELECT, and hydrated onto ShardState — and written by +/// nothing, in any repo. Every read returned NULL, always. They are gone; what is left is a pin that +/// removing two columns from the middle of a positional selector did not shift the ordinals of the +/// failure_* columns that follow them. +/// +public class Bug_5173_behind_threshold_columns_removed: OneOffConfigurationsContext +{ + [Fact] + public async Task the_threshold_columns_are_no_longer_created() + { + StoreOptions(x => x.Events.EnableExtendedProgressionTracking = true); + await theStore.EnsureStorageExistsAsync(typeof(IEvent)); + + var columns = await progressionColumnsAsync(); + + columns.ShouldNotContain("warning_behind_threshold"); + columns.ShouldNotContain("critical_behind_threshold"); + + // ...and the eight columns that carry something are untouched. + foreach (var expected in new[] + { + "heartbeat", "agent_status", "pause_reason", "running_on_node", "failure_category", + "failure_event_sequence", "failure_event_type", "failure_event_tenant_id" + }) + { + columns.ShouldContain(expected); + } + } + + private async Task> progressionColumnsAsync() + { + await using var conn = new NpgsqlConnection(ConnectionSource.ConnectionString); + await conn.OpenAsync(TestContext.Current.CancellationToken); + + await using var reader = await conn + .CreateCommand( + "select column_name from information_schema.columns where table_schema = :schema and table_name = 'mt_event_progression'") + .With("schema", theStore.Events.DatabaseSchemaName, NpgsqlTypes.NpgsqlDbType.Varchar) + .ExecuteReaderAsync(TestContext.Current.CancellationToken); + + var names = new List(); + while (await reader.ReadAsync(TestContext.Current.CancellationToken)) + { + names.Add(reader.GetString(0)); + } + + return names; + } +} diff --git a/src/Marten/Events/Daemon/Progress/ProjectionProgressStatement.cs b/src/Marten/Events/Daemon/Progress/ProjectionProgressStatement.cs index acb3e3cae8..34397b7788 100644 --- a/src/Marten/Events/Daemon/Progress/ProjectionProgressStatement.cs +++ b/src/Marten/Events/Daemon/Progress/ProjectionProgressStatement.cs @@ -41,7 +41,7 @@ protected override void configure(ICommandBuilder builder) // #5048 / jasperfx#565: the failure_* columns trail the existing extended block so the ordinals // ShardStateSelector walks stay stable. const string extendedColumns = - "heartbeat, agent_status, pause_reason, running_on_node, warning_behind_threshold, critical_behind_threshold, failure_category, failure_event_sequence, failure_event_type, failure_event_tenant_id"; + "heartbeat, agent_status, pause_reason, running_on_node, failure_category, failure_event_sequence, failure_event_type, failure_event_tenant_id"; if (_events.UseOptimizedProjectionRebuilds && _events.EnableExtendedProgressionTracking) { diff --git a/src/Marten/Events/Daemon/Progress/ShardStateSelector.cs b/src/Marten/Events/Daemon/Progress/ShardStateSelector.cs index 35d1eb99cc..9c3dc54a66 100644 --- a/src/Marten/Events/Daemon/Progress/ShardStateSelector.cs +++ b/src/Marten/Events/Daemon/Progress/ShardStateSelector.cs @@ -72,17 +72,9 @@ public async Task ResolveAsync(DbDataReader reader, CancellationToke } nextIndex++; - if (!await reader.IsDBNullAsync(nextIndex, token).ConfigureAwait(false)) - { - state.WarningBehindThreshold = await reader.GetFieldValueAsync(nextIndex, token).ConfigureAwait(false); - } - nextIndex++; - - if (!await reader.IsDBNullAsync(nextIndex, token).ConfigureAwait(false)) - { - state.CriticalBehindThreshold = await reader.GetFieldValueAsync(nextIndex, token).ConfigureAwait(false); - } - nextIndex++; + // #5173: WarningBehindThreshold / CriticalBehindThreshold were hydrated here from two + // columns nothing ever wrote. The properties remain on JasperFx's ShardState (unassigned + // and unread there too) until they are removed upstream. // #5048 / jasperfx#565: rehydrate the classified failure so a consumer polling the database // (CritterWatch when the publishing node is down) gets the same shape as a live ShardState diff --git a/src/Marten/Events/Schema/EventProgressionTable.cs b/src/Marten/Events/Schema/EventProgressionTable.cs index ff1b28cb84..32c62cde9f 100644 --- a/src/Marten/Events/Schema/EventProgressionTable.cs +++ b/src/Marten/Events/Schema/EventProgressionTable.cs @@ -50,8 +50,14 @@ public EventProgressionTable(EventGraph eventGraph): base(new PostgresqlObjectNa AddColumn("agent_status", "varchar(20)").AllowNulls(); AddColumn("pause_reason", "text").AllowNulls(); AddColumn("running_on_node", "integer").AllowNulls(); - AddColumn("warning_behind_threshold", "bigint").AllowNulls(); - AddColumn("critical_behind_threshold", "bigint").AllowNulls(); + + // #5173: warning_behind_threshold / critical_behind_threshold used to be created here. + // They were created, selected and hydrated onto ShardState -- and written by nothing, in + // any repo, so every read of them returned NULL. Two columns of storage, two entries in + // every extended-tracking SELECT and two selector ordinals, carrying nothing. Removed + // rather than wired: nothing ever owned the value. Existing deployments get an + // `alter table ... drop column` on their next apply, which is lossless because the + // columns were provably always NULL. // #5048 / jasperfx#565: the classified reason this shard is paused or stopped, so a consumer // polling the database (CritterWatch when the publishing node is DOWN, which is exactly when