From ac233eee77a0a7190ed9a470fdff9d1e0c065495 Mon Sep 17 00:00:00 2001 From: geekypunk Date: Sun, 9 Aug 2026 08:55:49 -0500 Subject: [PATCH 1/2] fix(postgres): bind every placeholder in the table-stats query MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- .../PostgresIntrospectionProvider.java | 30 ++++++++++------ .../PostgresIntrospectionProviderTest.java | 36 +++++++++++++++++++ 2 files changed, 56 insertions(+), 10 deletions(-) diff --git a/backend/src/main/java/com/dbaagent/provider/postgres/PostgresIntrospectionProvider.java b/backend/src/main/java/com/dbaagent/provider/postgres/PostgresIntrospectionProvider.java index b01a83b..592bf8b 100644 --- a/backend/src/main/java/com/dbaagent/provider/postgres/PostgresIntrospectionProvider.java +++ b/backend/src/main/java/com/dbaagent/provider/postgres/PostgresIntrospectionProvider.java @@ -223,21 +223,31 @@ public TableStats getTableStats(Connection connection, String database, String t TableStats stats = new TableStats(); stats.setTableName(tableName); + // One placeholder, resolved once in a CTE, instead of repeating `?::regclass` + // in every expression. The previous form had NINE placeholders — the two + // subtractions use two each — while the binding loop ran `i <= 7`, so + // parameters 8 and 9 were never set and every call threw + // `No value specified for parameter 8`. That silently broke table-growth + // snapshots for every table on every Postgres connection + // (TableGrowthMonitoringService logs it per table and carries on). + // + // Counting placeholders by hand is exactly what failed here, so the count is + // now impossible to get wrong: bind one value and reference it by name. String query = """ + WITH t AS (SELECT ?::regclass AS rel) SELECT - pg_size_pretty(pg_total_relation_size(?::regclass)) as total_size, - pg_total_relation_size(?::regclass) as total_bytes, - pg_size_pretty(pg_relation_size(?::regclass)) as data_size, - pg_relation_size(?::regclass) as data_bytes, - pg_size_pretty(pg_total_relation_size(?::regclass) - pg_relation_size(?::regclass)) as index_size, - (pg_total_relation_size(?::regclass) - pg_relation_size(?::regclass)) as index_bytes, - obj_description(?::regclass, 'pg_class') as comment + pg_size_pretty(pg_total_relation_size(rel)) as total_size, + pg_total_relation_size(rel) as total_bytes, + pg_size_pretty(pg_relation_size(rel)) as data_size, + pg_relation_size(rel) as data_bytes, + pg_size_pretty(pg_total_relation_size(rel) - pg_relation_size(rel)) as index_size, + (pg_total_relation_size(rel) - pg_relation_size(rel)) as index_bytes, + obj_description(rel, 'pg_class') as comment + FROM t """; try (PreparedStatement stmt = connection.prepareStatement(query)) { - for (int i = 1; i <= 7; i++) { - stmt.setString(i, tableName); - } + stmt.setString(1, tableName); try (ResultSet rs = stmt.executeQuery()) { if (rs.next()) { diff --git a/backend/src/test/java/com/dbaagent/provider/postgres/PostgresIntrospectionProviderTest.java b/backend/src/test/java/com/dbaagent/provider/postgres/PostgresIntrospectionProviderTest.java index 57966a9..2e7ea5c 100644 --- a/backend/src/test/java/com/dbaagent/provider/postgres/PostgresIntrospectionProviderTest.java +++ b/backend/src/test/java/com/dbaagent/provider/postgres/PostgresIntrospectionProviderTest.java @@ -10,8 +10,12 @@ import java.sql.*; import java.util.List; +import org.mockito.ArgumentCaptor; + import static org.junit.jupiter.api.Assertions.*; +import static org.mockito.ArgumentMatchers.anyInt; import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.*; @ExtendWith(MockitoExtension.class) @@ -171,6 +175,38 @@ void getTableStats_returnsStats() throws SQLException { assertEquals(122880L, stats.getSizeBytes()); } + @Test + void getTableStats_bindsEveryPlaceholderInTheStatsQuery() throws SQLException { + // The stats query carried NINE `?` placeholders (the two size subtractions use + // two each) while the binding loop ran `i <= 7`, so parameters 8 and 9 were + // never set and Postgres rejected every call with + // No value specified for parameter 8 + // silently killing table-growth snapshots for every table. + // + // getTableStats_returnsStats above passed throughout, because a mocked + // PreparedStatement does not enforce that placeholders are bound. This test + // compares the two directly, so the count can never drift again. + PreparedStatement rowCountStatement = mock(PreparedStatement.class); + ResultSet rowCountResultSet = mock(ResultSet.class); + ArgumentCaptor sqlCaptor = ArgumentCaptor.forClass(String.class); + + when(connection.prepareStatement(anyString())).thenReturn(preparedStatement, rowCountStatement); + when(preparedStatement.executeQuery()).thenReturn(resultSet); + when(rowCountStatement.executeQuery()).thenReturn(rowCountResultSet); + when(resultSet.next()).thenReturn(true); + when(rowCountResultSet.next()).thenReturn(true); + when(rowCountResultSet.getObject("row_count")).thenReturn(1000L); + + provider.getTableStats(connection, "public", "users"); + + verify(connection, atLeastOnce()).prepareStatement(sqlCaptor.capture()); + String statsQuery = sqlCaptor.getAllValues().get(0); + int placeholders = (int) statsQuery.chars().filter(c -> c == '?').count(); + + assertTrue(placeholders > 0, "stats query should still be parameterised"); + verify(preparedStatement, times(placeholders)).setString(anyInt(), eq("users")); + } + @Test void scanSchema_returnsSchemaMetadata() throws SQLException { when(connection.createStatement()).thenReturn(statement); From b4d9d41796c2c877ff2405db233aae5dfe34c0a0 Mon Sep 17 00:00:00 2001 From: geekypunk Date: Sun, 9 Aug 2026 08:56:00 -0500 Subject: [PATCH 2/2] fix(scheduler): revive dead brain-init executions in ~6m instead of 30m MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- backend/src/main/resources/application.properties | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/backend/src/main/resources/application.properties b/backend/src/main/resources/application.properties index 672453a..3ef9e6d 100644 --- a/backend/src/main/resources/application.properties +++ b/backend/src/main/resources/application.properties @@ -71,7 +71,19 @@ spring.batch.job.enabled=false # db-scheduler Configuration (distributed task scheduling) db-scheduler.enabled=true -db-scheduler.heartbeat-interval=5m +# The revival window for an execution whose owner died is +# heartbeat-interval × missed-heartbeats-limit. At the old 5m × 6 that was THIRTY +# MINUTES, during which a dead brain-init stage sits at a frozen percentage with no +# error: the status endpoint keeps reporting the last stage it reached, so it is +# indistinguishable from slow progress. smoke-test.sh gives up after 1200s (20m), +# so any run where a stage died failed the smoke test even though init would have +# completed fine once revived — observed exactly that, init finishing ~37m after +# revival. +# +# 1m × 6 keeps the same six-missed-heartbeat tolerance — a live-but-slow execution +# heartbeats from its own thread, so a long LLM call is never mistaken for death — +# while bringing revival to ~6m, comfortably inside the smoke-test window. +db-scheduler.heartbeat-interval=1m db-scheduler.missed-heartbeats-limit=6 db-scheduler.polling-interval=10s db-scheduler.polling-strategy=fetch