Skip to content

Commit 427480a

Browse files
Merge branch 'main' into dependabot/npm_and_yarn/mcp/inquirer/prompts-8.7.0
2 parents 05c19f5 + 45d0908 commit 427480a

16 files changed

Lines changed: 516 additions & 43 deletions
Lines changed: 166 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,166 @@
1+
package com.dbaagent.config;
2+
3+
import lombok.extern.slf4j.Slf4j;
4+
import org.springframework.context.annotation.Bean;
5+
import org.springframework.context.annotation.Configuration;
6+
import org.springframework.context.annotation.DependsOn;
7+
import org.springframework.jdbc.core.JdbcTemplate;
8+
9+
import javax.sql.DataSource;
10+
11+
/**
12+
* Adds the stored normalized column and index that make slow-query sample recovery
13+
* indexable.
14+
*
15+
* <p>{@code QueryLineageRepository.findLongestByConnectionIdAndNormalizedQueryTextPrefix}
16+
* wraps {@code query_text} in three nested {@code regexp_replace}/{@code REPLACE} calls
17+
* plus {@code LOWER} before comparing it, so no index on {@code query_text} can ever
18+
* satisfy the predicate — Postgres must materialize a rewritten copy of every row in the
19+
* connection's slice. {@code EXPLAIN (ANALYZE)} on a real install:
20+
*
21+
* <pre>
22+
* Seq Scan on query_lineage (actual time=0.244..35.826 rows=422 loops=1)
23+
* Execution Time: 36.179 ms -- at 1,093 rows
24+
* Execution Time: 1111.996 ms -- same query, table scaled to 34,976 rows
25+
* </pre>
26+
*
27+
* <p>{@code SlowQueryAnalyticsService.recoverFullText} runs that once per sample, up to
28+
* 20 per "view full query" click, so one modal open costs ~0.7 s today and ~22 s at 35k
29+
* rows. It degrades with <em>age</em> rather than load, which is why it passes every
30+
* pre-launch test: {@code query_lineage} is not pruned by
31+
* {@code SlowQueryRetentionService} (that only touches {@code slow_query_run},
32+
* {@code slow_query_customer_day} and {@code slow_query_sample}), so it only grows.
33+
*
34+
* <p>Precomputing the normalization into a STORED generated column pays the regex chain
35+
* once at write time. Measured on the same scaled table, with the ~120-character prefix
36+
* the caller actually sends:
37+
*
38+
* <pre>
39+
* Index Scan using idx_query_lineage_norm_match
40+
* Execution Time: 0.428 ms -- vs 1111.996 ms
41+
* </pre>
42+
*
43+
* <p>The index earns its keep only because the prefix is long and therefore selective. A
44+
* short prefix such as {@code 'select%'} still plans as a sequential scan (~50 ms at 35k
45+
* rows) — that is the precomputation alone, and is fine. Do not "simplify" this by
46+
* dropping the generated column and indexing {@code query_text} directly; the expression,
47+
* not the column, is what the query compares.
48+
*
49+
* <p>There is no Flyway runtime in this repo (see CLAUDE.md), so this initializer is what
50+
* actually applies {@code V118__add_query_lineage_norm_match.sql}. Both statements are
51+
* {@code IF NOT EXISTS} and the whole thing is best-effort: a failure here costs
52+
* performance, never correctness, since the query returns identical rows either way.
53+
*/
54+
@Configuration
55+
@Slf4j
56+
public class QueryLineageMatchIndexInitializer {
57+
58+
private static final String TABLE = "query_lineage";
59+
private static final String COLUMN = "normalized_match";
60+
private static final String INDEX = "idx_query_lineage_norm_match";
61+
62+
/**
63+
* Must match {@code SlowQueryAnalyticsService.normalizeForMatching} exactly. If one
64+
* changes, both do.
65+
*
66+
* <p>The {@code btrim} is load-bearing and was missing from the expression this
67+
* replaces: Java's {@code normalizeForMatching} ends with {@code .trim()}, so a
68+
* lineage row stored with leading whitespace normalized to {@code " select ..."} on
69+
* the SQL side and {@code "select ..."} on the Java side. The prefix {@code LIKE}
70+
* then never matched and recovery silently returned the truncated sample. 4 of 1,174
71+
* rows on the local install carry such whitespace.
72+
*/
73+
private static final String NORMALIZE_EXPR =
74+
"btrim(lower(regexp_replace(regexp_replace("
75+
+ "replace(query_text, '`', ''), "
76+
+ "'\\s*([.,();])\\s*', '\\1', 'g'), "
77+
+ "'\\s+', ' ', 'g')))";
78+
79+
@Bean("queryLineageMatchIndexBootstrap")
80+
@DependsOn("entityManagerFactory")
81+
public Object queryLineageMatchIndexBootstrap(DataSource dataSource) {
82+
JdbcTemplate jdbc = new JdbcTemplate(dataSource);
83+
84+
if (!tableExists(jdbc, TABLE)) {
85+
return new Object();
86+
}
87+
88+
// A generated column's expression cannot be altered in place, and ADD COLUMN IF
89+
// NOT EXISTS silently keeps whatever definition is already there. An install that
90+
// ran an earlier build of this initializer therefore keeps the untrimmed
91+
// expression forever unless the column is dropped first. Only drop when the
92+
// definition actually differs, so a normal restart does not rewrite the table.
93+
if (columnDefinitionDiffers(jdbc)) {
94+
log.info("Rebuilding {}.{}: stored expression is out of date", TABLE, COLUMN);
95+
try {
96+
jdbc.execute("ALTER TABLE " + TABLE + " DROP COLUMN " + COLUMN);
97+
} catch (RuntimeException e) {
98+
log.warn("Could not drop stale {}.{}: {}", TABLE, COLUMN, e.getMessage());
99+
}
100+
}
101+
102+
try {
103+
jdbc.execute("ALTER TABLE " + TABLE + " ADD COLUMN IF NOT EXISTS " + COLUMN
104+
+ " text GENERATED ALWAYS AS (" + NORMALIZE_EXPR + ") STORED");
105+
} catch (RuntimeException e) {
106+
// Generated columns need Postgres 12+. Older servers keep the sequential scan,
107+
// which is slow but correct, so this must not stop the application.
108+
log.warn("Could not add {}.{} ({}); sample recovery stays on a sequential scan",
109+
TABLE, COLUMN, e.getMessage());
110+
return new Object();
111+
}
112+
113+
try {
114+
// text_pattern_ops so a LIKE 'prefix%' comparison can use the index under any
115+
// collation; the default opclass only helps in the C collation.
116+
jdbc.execute("CREATE INDEX IF NOT EXISTS " + INDEX + " ON " + TABLE
117+
+ " (connection_id, " + COLUMN + " text_pattern_ops)");
118+
} catch (RuntimeException e) {
119+
log.warn("Could not create {}: {}", INDEX, e.getMessage());
120+
}
121+
122+
return new Object();
123+
}
124+
125+
/**
126+
* True when {@code normalized_match} exists but was generated by a different
127+
* expression than {@link #NORMALIZE_EXPR}. Compared on the normalized form Postgres
128+
* stores in {@code pg_get_expr}, with whitespace collapsed, since the server rewrites
129+
* the text it was given (adds casts, reorders parens) and a literal comparison would
130+
* report a difference on every start and rewrite the table each time.
131+
*/
132+
private static boolean columnDefinitionDiffers(JdbcTemplate jdbc) {
133+
try {
134+
String stored = jdbc.query(
135+
"SELECT pg_get_expr(d.adbin, d.adrelid) "
136+
+ "FROM pg_attrdef d "
137+
+ "JOIN pg_attribute a ON a.attrelid = d.adrelid AND a.attnum = d.adnum "
138+
+ "WHERE d.adrelid = ?::regclass AND a.attname = ?",
139+
rs -> rs.next() ? rs.getString(1) : null, TABLE, COLUMN);
140+
if (stored == null) {
141+
return false; // column not present yet — nothing stale to drop
142+
}
143+
return !squash(stored).contains("btrim");
144+
} catch (RuntimeException e) {
145+
log.warn("Could not inspect {}.{} definition: {}", TABLE, COLUMN, e.getMessage());
146+
return false;
147+
}
148+
}
149+
150+
private static String squash(String s) {
151+
return s.replaceAll("\\s+", "").toLowerCase();
152+
}
153+
154+
private static boolean tableExists(JdbcTemplate jdbc, String table) {
155+
try {
156+
Integer found = jdbc.queryForObject(
157+
"SELECT COUNT(*) FROM information_schema.tables "
158+
+ "WHERE table_schema = current_schema() AND table_name = ?",
159+
Integer.class, table);
160+
return found != null && found > 0;
161+
} catch (RuntimeException e) {
162+
log.warn("Could not check for table {}: {}", table, e.getMessage());
163+
return false;
164+
}
165+
}
166+
}

backend/src/main/java/com/dbaagent/controller/SlowQueryAnalyticsController.java

Lines changed: 39 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,41 @@ public ResponseEntity<List<SlowQueryAnalyticsService.CustomerSummary>> listCusto
7878
return ResponseEntity.ok(analyticsService.listCustomers(connectionId));
7979
}
8080

81-
/** Every slow query attributed to one customer, ranked by their mean exec time. */
81+
/**
82+
* Every slow query attributed to one customer, ranked by their mean exec time.
83+
*
84+
* <p>The customer id is a <b>query parameter</b>, not a path segment, because it is a
85+
* literal value read out of the tenant column — application data, which can contain
86+
* {@code /}, {@code ?} or {@code #}. Such an id is unreachable as a path segment under
87+
* any encoding: raw, the slash splits the path; percent-encoded, Jetty rejects it with
88+
* {@code 400 Ambiguous URI path separator}. Both were reproduced against this backend
89+
* with the tenant value {@code acct/77?x=1}, whose rows were simply invisible in the
90+
* By-Customer view.
91+
*/
92+
@GetMapping("/{connectionId}/customer-queries")
93+
public ResponseEntity<List<SlowQueryAnalyticsService.CustomerQueryRow>> customerQueries(
94+
@PathVariable String connectionId,
95+
@RequestParam String customerId) {
96+
accessControlService.assertCanReadConnectionContent(connectionId);
97+
return ResponseEntity.ok(analyticsService.queriesForCustomer(connectionId, customerId));
98+
}
99+
100+
/** Literal-bearing samples for one (customer, query) pair — copyable SQL. */
101+
@GetMapping("/{connectionId}/customer-query-samples")
102+
public ResponseEntity<List<SlowQueryAnalyticsService.QuerySample>> customerQuerySamples(
103+
@PathVariable String connectionId,
104+
@RequestParam String customerId,
105+
@RequestParam String fingerprint) {
106+
accessControlService.assertCanReadConnectionContent(connectionId);
107+
return ResponseEntity.ok(
108+
analyticsService.samplesForCustomerQuery(connectionId, customerId, fingerprint));
109+
}
110+
111+
/**
112+
* @deprecated superseded by {@link #customerQueries}; a customer id containing a
113+
* slash cannot be expressed here. Retained so existing clients keep working.
114+
*/
115+
@Deprecated
82116
@GetMapping("/{connectionId}/customer/{customerId}/queries")
83117
public ResponseEntity<List<SlowQueryAnalyticsService.CustomerQueryRow>> queriesForCustomer(
84118
@PathVariable String connectionId,
@@ -87,7 +121,10 @@ public ResponseEntity<List<SlowQueryAnalyticsService.CustomerQueryRow>> queriesF
87121
return ResponseEntity.ok(analyticsService.queriesForCustomer(connectionId, customerId));
88122
}
89123

90-
/** Literal-bearing samples for one (customer, query) pair — copyable SQL. */
124+
/**
125+
* @deprecated superseded by {@link #customerQuerySamples}; see above.
126+
*/
127+
@Deprecated
91128
@GetMapping("/{connectionId}/customer/{customerId}/query/{fingerprint}/samples")
92129
public ResponseEntity<List<SlowQueryAnalyticsService.QuerySample>> samplesForCustomerQuery(
93130
@PathVariable String connectionId,

backend/src/main/java/com/dbaagent/controller/WorkloadAnalysisController.java

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,12 @@
2727
@Slf4j
2828
public class WorkloadAnalysisController {
2929

30+
// Reads use canReadContent; only `run` requires canManageContent. The reads were all
31+
// gated on manage, which is the write tier — EffectiveConnectionAccess's own comment
32+
// lists slow-query analytics under read. Latent today because every grant resolves to
33+
// FULL_CONTENT (so both predicates are true), but it would deny the whole Workload tab
34+
// to a read-only grant the moment one is reintroduced.
35+
3036
private final WorkloadAnalysisService workloadAnalysisService;
3137
private final WorkloadAnalysisReportRepository reportRepository;
3238
private final AccessControlService accessControlService;
@@ -64,7 +70,7 @@ public ResponseEntity<Map<String, Object>> run(@PathVariable String connectionId
6470
/** Lightweight poll payload — status + progress without the full report blob. */
6571
@GetMapping("/{connectionId}/status")
6672
public ResponseEntity<Map<String, Object>> status(@PathVariable String connectionId) {
67-
accessControlService.assertCanManageConnectionContent(connectionId);
73+
accessControlService.assertCanReadConnectionContent(connectionId);
6874
return reportRepository.findFirstByConnectionIdOrderByStartedAtDesc(connectionId)
6975
.map(r -> ResponseEntity.ok(Map.<String, Object>of(
7076
"reportId", r.getId(),
@@ -81,7 +87,7 @@ public ResponseEntity<Map<String, Object>> status(@PathVariable String connectio
8187
/** The full latest report (with the composed sections). 204 if none yet. */
8288
@GetMapping("/{connectionId}/latest")
8389
public ResponseEntity<WorkloadAnalysisReport> latest(@PathVariable String connectionId) {
84-
accessControlService.assertCanManageConnectionContent(connectionId);
90+
accessControlService.assertCanReadConnectionContent(connectionId);
8591
return reportRepository.findFirstByConnectionIdOrderByStartedAtDesc(connectionId)
8692
.map(ResponseEntity::ok)
8793
.orElseGet(() -> ResponseEntity.noContent().build());
@@ -92,7 +98,7 @@ public ResponseEntity<WorkloadAnalysisReport> latest(@PathVariable String connec
9298
public ResponseEntity<WorkloadAnalysisReport> getReport(@PathVariable String reportId) {
9399
return reportRepository.findById(reportId)
94100
.map(r -> {
95-
accessControlService.assertCanManageConnectionContent(r.getConnectionId());
101+
accessControlService.assertCanReadConnectionContent(r.getConnectionId());
96102
return ResponseEntity.ok(r);
97103
})
98104
.orElseGet(() -> ResponseEntity.notFound().build());
@@ -101,7 +107,7 @@ public ResponseEntity<WorkloadAnalysisReport> getReport(@PathVariable String rep
101107
/** Recent report history (newest first), metadata only via the entity. */
102108
@GetMapping("/{connectionId}/history")
103109
public ResponseEntity<List<WorkloadAnalysisReport>> history(@PathVariable String connectionId) {
104-
accessControlService.assertCanManageConnectionContent(connectionId);
110+
accessControlService.assertCanReadConnectionContent(connectionId);
105111
return ResponseEntity.ok(reportRepository.findTop20ByConnectionIdOrderByStartedAtDesc(connectionId));
106112
}
107113
}

backend/src/main/java/com/dbaagent/repository/QueryLineageRepository.java

Lines changed: 43 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,10 @@
33
import com.dbaagent.model.QueryLineage;
44
import org.springframework.data.jpa.repository.JpaRepository;
55
import org.springframework.data.jpa.repository.Query;
6+
import org.springframework.data.jpa.repository.Modifying;
67
import org.springframework.data.domain.Pageable;
78
import org.springframework.data.repository.query.Param;
9+
import org.springframework.transaction.annotation.Transactional;
810
import org.springframework.stereotype.Repository;
911

1012
import java.time.LocalDateTime;
@@ -147,23 +149,32 @@ QueryLineage findLongestByConnectionIdAndCollapsedQueryTextPrefix(
147149
* Callers must apply the SAME transformation to their prefix on the
148150
* Java side (see {@code SlowQueryAnalyticsService.normalizeForMatching}).
149151
*
150-
* <p>The regex chain runs once per candidate row, so this is intended for
151-
* single-row lookups (LIMIT 1) over the per-connection slice. For
152-
* d840f866-style connections (~5K lineage rows) it returns in a few ms;
153-
* larger connections may benefit from a functional index on the
154-
* normalized expression, but none is needed yet.
152+
* <p>The normalization is <b>precomputed</b> into the stored generated column
153+
* {@code normalized_match} ({@code QueryLineageMatchIndexInitializer}) and matched
154+
* against that, so the regex chain runs once at write time rather than once per
155+
* candidate row on every read. Inlining the expression here made the predicate
156+
* unindexable: Postgres had to materialize a rewritten copy of the whole
157+
* per-connection slice, which measured 36 ms at 1,093 rows and 1,112 ms at 34,976 —
158+
* and {@code recoverFullText} issues this up to 20 times per "view full query" click,
159+
* against a table no retention job prunes. On the same scaled table with the ~120-char
160+
* prefix the caller actually sends, the indexed form plans as an Index Scan at
161+
* 0.428 ms.
162+
*
163+
* <p>Matched against the column <b>directly</b>, with no {@code COALESCE} fallback to
164+
* the inline expression. That fallback is the obvious way to stay safe on a database
165+
* without the column, and it silently undoes the whole fix: wrapping the column in
166+
* {@code COALESCE(...)} makes the predicate non-indexable again. Measured on the same
167+
* 37,504-row table — {@code COALESCE(normalized_match, …)} plans a Seq Scan at
168+
* 48.7 ms, the bare column an Index Scan at 0.39 ms. If the column is ever absent,
169+
* {@code QueryLineageMatchIndexInitializer} logs it at WARN and
170+
* {@code SlowQueryAnalyticsService.recoverFullText} already treats a failed lookup as
171+
* "no longer text available" and returns the sample unchanged.
155172
*/
156173
@Query(value = """
157174
SELECT * FROM query_lineage
158175
WHERE connection_id = :connectionId
159176
AND LENGTH(query_text) > :minLength
160-
AND LOWER(
161-
regexp_replace(
162-
regexp_replace(
163-
REPLACE(query_text, '`', ''),
164-
'\\s*([.,();])\\s*', '\\1', 'g'),
165-
'\\s+', ' ', 'g')
166-
) LIKE :escapedPrefix ESCAPE '\\'
177+
AND normalized_match LIKE :escapedPrefix ESCAPE '\\'
167178
ORDER BY LENGTH(query_text) DESC
168179
LIMIT 1
169180
""", nativeQuery = true)
@@ -172,4 +183,24 @@ QueryLineage findLongestByConnectionIdAndNormalizedQueryTextPrefix(
172183
@Param("escapedPrefix") String escapedPrefix,
173184
@Param("minLength") int minLength
174185
);
186+
187+
/**
188+
* Drop lineage rows older than the connection's retention window.
189+
*
190+
* <p>This table was never pruned: {@code SlowQueryRetentionService} covered
191+
* {@code slow_query_run}, {@code slow_query_customer_day} and
192+
* {@code slow_query_sample} but not lineage, so it grew without bound while the
193+
* 30-day analytics tables stayed small. That is what made sample recovery degrade
194+
* with age rather than load — the scanned table kept growing even on an idle install.
195+
*
196+
* <p>Keyed on {@code created_at}, which is non-null and already indexed
197+
* ({@code idx_query_lineage_created}).
198+
*/
199+
@Modifying
200+
@Transactional
201+
@Query("DELETE FROM QueryLineage q WHERE q.connectionId = :connectionId "
202+
+ "AND q.createdAt < :cutoff")
203+
int deleteByConnectionIdAndCreatedAtBefore(
204+
@Param("connectionId") String connectionId,
205+
@Param("cutoff") java.time.LocalDateTime cutoff);
175206
}

0 commit comments

Comments
 (0)