Keep the behind_threshold columns so upgrading does not change this schema (#5173)

[jeremydmiller]

Partially reverts #5184. The read-side half of that change stands — warning_behind_threshold / critical_behind_threshold are still not selected and not hydrated onto ShardState, and ShardStateSelector's ordinals still skip them. Only the DDL is restored, so the two columns exist and carry nothing.

Why

#5184's reasoning about the data holds: nothing in any repo ever wrote or read these, every read returned NULL, and every lag threshold lives in the monitoring tool. What it got wrong was the upgrade cost. Its commit message says existing deployments "get an alter table ... drop column on their next ApplyAllConfiguredChangesToDatabaseAsync(), which is lossless."

Measured against PostgreSQL — on a store with event sourcing registered, with an mt_event_progression carrying a column the configuration no longer declares:

CreateMigrationAsync().Difference           = Update   (mt_event_progression=Update)
AssertDatabaseMatchesConfigurationAsync()   -> throws
ApplyAllConfiguredChangesToDatabaseAsync()  -> drops the column

Two consequences for existing deployments, neither of them "lossless in passing":

1. A host running AutoCreate.None with a schema assertion at boot — the standard production posture — does not migrate. It refuses to start until someone migrates it.
2. A host that does apply runs alter table … drop column. In PostgreSQL that is O(1) metadata, but it needs ACCESS EXCLUSIVE on a small, hot table that every running daemon writes. The operation is instant; acquiring the lock is not, and while it waits it blocks every reader and writer queued behind it.

Two always-NULL columns are much cheaper than asking every deployment to absorb that. Keeping them also means a freshly created database and an upgraded one have the same shape, rather than diverging on a detail nothing reads.

If they are ever removed for real, it should be a documented migration step — not a silent consequence of upgrading a package.

A note on the tests

Both new tests fail with the columns taken back out. The AddEventType in each is load-bearing:

Without an event type registered, the event store's tables are not part of the schema migration at all — every delta comes back None and any assertion about mt_event_progression passes vacuously.

That is exactly how the first version of this test was green against both shapes, and it is the same feature-scoping trap that produced a wrong diagnosis on CritterWatch#889. Worth knowing about for anything else that asserts on schema deltas.

Verification

• DaemonTests Bug_5173_* — 2/2 green with the restore, 2/2 red without it.
• Found while proving the unreleased Marten/JasperFx packs against CritterWatch. Companion release blocker in the same batch: ExtendedProgressionWriter: don't lose the first status transition, and say when a duplicate writer attaches jasperfx#631.

🤖 Generated with Claude Code

https://claude.ai/code/session_016v2Aijyo8MX2AdPUZL5VtG