Skip to content

fix: table-growth snapshots (unbound SQL parameter) and slow brain-init revival - #39

Merged
geekypunk merged 2 commits into
mainfrom
fix/growth-snapshot-and-brain-init-revival
Aug 9, 2026
Merged

fix: table-growth snapshots (unbound SQL parameter) and slow brain-init revival#39
geekypunk merged 2 commits into
mainfrom
fix/growth-snapshot-and-brain-init-revival

Conversation

@geekypunk

Copy link
Copy Markdown
Contributor

Two independent defects found while running the self-host install flow end to end. Both were silent — each one degraded a feature while the surrounding job reported success.

1. Table-growth snapshots never captured on Postgres

PostgresIntrospectionProvider.getTableStats built a query with nine ? placeholders (the two size subtractions use two each) while the binding loop ran for (i = 1; i <= 7). Parameters 8 and 9 were never set, so every call failed with:

No value specified for parameter 8

TableGrowthMonitoringService logs this per table and carries on, so the scheduled job "succeeded" while capturing nothing — for every table, on every Postgres connection.

The query now binds a single value and references it through a CTE, so the count cannot drift again.

Verified against a live Postgres 18: the old form reports 9 parameters via pg_prepared_statements; the new one reports 1 and returns correct sizes for dba_batch_job_execution, the exact table from the log. MySQL's getTableStats was checked and is unaffected (2 placeholders, 2 bound).

2. Dead brain-init stages took 30 minutes to revive

The reclaim window is heartbeat-interval × missed-heartbeats-limit = 5m × 6 = 30 minutes.

During it a dead stage is invisible rather than failed: init-status keeps returning the last stage and percentage, with completedAt and errorMessage both null — a frozen run looks exactly like a slow one. smoke-test.sh waits 1200s (20m), less than the revival window, so any run where a stage died failed the smoke test even though initialization would have completed normally.

Observed exactly that: a stage stopped heartbeating, the smoke test timed out at 74%, and brain init then reached 100% about 37 minutes after the scheduler revived it.

1m × 6 keeps the same six-missed-heartbeat tolerance and brings revival to ~6 minutes, inside the smoke-test window. Shortening the interval does not risk reclaiming healthy work — heartbeats come from the execution's own thread, so a long LLM call keeps heartbeating.

Testing

  • PostgresIntrospectionProviderTest — 11/11 pass.
  • The new test compares placeholder count against bind count. It fails against the pre-fix provider with Wanted 9 times, while the other 10 tests still pass — which is why this reached production: getTableStats_returnsStats passed throughout, because a mocked PreparedStatement does not enforce that placeholders are bound.
  • application-test.properties overrides polling-interval and immediate-execution-enabled, not the heartbeat keys, so tests are unaffected by change 2.

Not addressed here

TableGrowthMonitoringService.captureSnapshot is @Transactional and private. Spring cannot proxy a private method, and both call sites reach it by self-invocation, so the annotation is inert — while its Javadoc claims it "ensures all repository operations use a single database connection, reducing connection pool pressure from 4-5 connections per table to just 1." That claim does not hold today. Left out of this PR because fixing it changes transaction boundaries and deserves its own review.

getTableStats built a query with NINE `?` placeholders — the two size
subtractions use two each — while the binding loop ran `for (i = 1; i <= 7)`.
Parameters 8 and 9 were never set, so Postgres rejected every call with

  No value specified for parameter 8

That silently disabled table-growth snapshots for every table on every Postgres
connection: TableGrowthMonitoringService logs the failure per table and carries
on, so the scheduled job "succeeded" while capturing nothing. It shows up in the
backend log as a steady stream of

  ✗ Failed to capture snapshot for table: dba_batch_job_execution - No value
    specified for parameter 8.

The query now binds one value and references it through a CTE, so the count
cannot drift again — counting placeholders by hand is precisely what failed here.
Verified against a live Postgres 18: the old form reports 9 parameters via
pg_prepared_statements, the new one reports 1 and returns correct sizes for
dba_batch_job_execution, the exact table from the log.

MySQL's getTableStats was checked and is unaffected (2 placeholders, 2 bound).

Tests: the existing getTableStats_returnsStats passed throughout the bug, because
a mocked PreparedStatement does not enforce that placeholders are bound. The new
test compares the two directly — it fails against the pre-fix provider with
"Wanted 9 times" while the other 10 tests still pass, which is exactly why this
reached production.
The window before db-scheduler reclaims an execution whose owner died is
heartbeat-interval × missed-heartbeats-limit. At 5m × 6 that was thirty minutes.

During it a dead brain-init stage is invisible rather than failed: the
init-status endpoint keeps returning the last stage and percentage it reached,
with completedAt null and errorMessage null, so a frozen run is indistinguishable
from a slow one. smoke-test.sh waits 1200s (20m) — less than the revival window —
so any run in which a stage died failed the smoke test even though initialization
would have finished normally once revived. Observed exactly that: a stage stopped
heartbeating, the smoke test timed out at 74%, and brain init then completed at
100% roughly 37 minutes after the scheduler revived it.

1m × 6 keeps the same six-missed-heartbeat tolerance and brings revival to about
six minutes, comfortably inside the smoke-test window. Shortening the interval
does not risk reclaiming healthy work: heartbeats are sent from the execution's
own thread, so a long-running LLM call keeps heartbeating and is never mistaken
for a dead owner.

Only the heartbeat interval changes. application-test.properties overrides
polling-interval and immediate-execution-enabled, not these keys, so tests are
unaffected.
@geekypunk
geekypunk merged commit 57eb925 into main Aug 9, 2026
7 checks passed
@geekypunk
geekypunk deleted the fix/growth-snapshot-and-brain-init-revival branch August 9, 2026 16:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant