Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,166 @@
package com.dbaagent.config;

import lombok.extern.slf4j.Slf4j;
import org.springframework.context.annotation.Bean;
import org.springframework.context.annotation.Configuration;
import org.springframework.context.annotation.DependsOn;
import org.springframework.jdbc.core.JdbcTemplate;

import javax.sql.DataSource;

/**
* Adds the stored normalized column and index that make slow-query sample recovery
* indexable.
*
* <p>{@code QueryLineageRepository.findLongestByConnectionIdAndNormalizedQueryTextPrefix}
* wraps {@code query_text} in three nested {@code regexp_replace}/{@code REPLACE} calls
* plus {@code LOWER} before comparing it, so no index on {@code query_text} can ever
* satisfy the predicate — Postgres must materialize a rewritten copy of every row in the
* connection's slice. {@code EXPLAIN (ANALYZE)} on a real install:
*
* <pre>
* Seq Scan on query_lineage (actual time=0.244..35.826 rows=422 loops=1)
* Execution Time: 36.179 ms -- at 1,093 rows
* Execution Time: 1111.996 ms -- same query, table scaled to 34,976 rows
* </pre>
*
* <p>{@code SlowQueryAnalyticsService.recoverFullText} runs that once per sample, up to
* 20 per "view full query" click, so one modal open costs ~0.7 s today and ~22 s at 35k
* rows. It degrades with <em>age</em> rather than load, which is why it passes every
* pre-launch test: {@code query_lineage} is not pruned by
* {@code SlowQueryRetentionService} (that only touches {@code slow_query_run},
* {@code slow_query_customer_day} and {@code slow_query_sample}), so it only grows.
*
* <p>Precomputing the normalization into a STORED generated column pays the regex chain
* once at write time. Measured on the same scaled table, with the ~120-character prefix
* the caller actually sends:
*
* <pre>
* Index Scan using idx_query_lineage_norm_match
* Execution Time: 0.428 ms -- vs 1111.996 ms
* </pre>
*
* <p>The index earns its keep only because the prefix is long and therefore selective. A
* short prefix such as {@code 'select%'} still plans as a sequential scan (~50 ms at 35k
* rows) — that is the precomputation alone, and is fine. Do not "simplify" this by
* dropping the generated column and indexing {@code query_text} directly; the expression,
* not the column, is what the query compares.
*
* <p>There is no Flyway runtime in this repo (see CLAUDE.md), so this initializer is what
* actually applies {@code V118__add_query_lineage_norm_match.sql}. Both statements are
* {@code IF NOT EXISTS} and the whole thing is best-effort: a failure here costs
* performance, never correctness, since the query returns identical rows either way.
*/
@Configuration
@Slf4j
public class QueryLineageMatchIndexInitializer {

private static final String TABLE = "query_lineage";
private static final String COLUMN = "normalized_match";
private static final String INDEX = "idx_query_lineage_norm_match";

/**
* Must match {@code SlowQueryAnalyticsService.normalizeForMatching} exactly. If one
* changes, both do.
*
* <p>The {@code btrim} is load-bearing and was missing from the expression this
* replaces: Java's {@code normalizeForMatching} ends with {@code .trim()}, so a
* lineage row stored with leading whitespace normalized to {@code " select ..."} on
* the SQL side and {@code "select ..."} on the Java side. The prefix {@code LIKE}
* then never matched and recovery silently returned the truncated sample. 4 of 1,174
* rows on the local install carry such whitespace.
*/
private static final String NORMALIZE_EXPR =
"btrim(lower(regexp_replace(regexp_replace("
+ "replace(query_text, '`', ''), "
+ "'\\s*([.,();])\\s*', '\\1', 'g'), "
+ "'\\s+', ' ', 'g')))";

@Bean("queryLineageMatchIndexBootstrap")
@DependsOn("entityManagerFactory")
public Object queryLineageMatchIndexBootstrap(DataSource dataSource) {
JdbcTemplate jdbc = new JdbcTemplate(dataSource);

if (!tableExists(jdbc, TABLE)) {
return new Object();
}

// A generated column's expression cannot be altered in place, and ADD COLUMN IF
// NOT EXISTS silently keeps whatever definition is already there. An install that
// ran an earlier build of this initializer therefore keeps the untrimmed
// expression forever unless the column is dropped first. Only drop when the
// definition actually differs, so a normal restart does not rewrite the table.
if (columnDefinitionDiffers(jdbc)) {
log.info("Rebuilding {}.{}: stored expression is out of date", TABLE, COLUMN);
try {
jdbc.execute("ALTER TABLE " + TABLE + " DROP COLUMN " + COLUMN);
} catch (RuntimeException e) {
log.warn("Could not drop stale {}.{}: {}", TABLE, COLUMN, e.getMessage());
}
}

try {
jdbc.execute("ALTER TABLE " + TABLE + " ADD COLUMN IF NOT EXISTS " + COLUMN
+ " text GENERATED ALWAYS AS (" + NORMALIZE_EXPR + ") STORED");
} catch (RuntimeException e) {
// Generated columns need Postgres 12+. Older servers keep the sequential scan,
// which is slow but correct, so this must not stop the application.
log.warn("Could not add {}.{} ({}); sample recovery stays on a sequential scan",
TABLE, COLUMN, e.getMessage());
return new Object();
}

try {
// text_pattern_ops so a LIKE 'prefix%' comparison can use the index under any
// collation; the default opclass only helps in the C collation.
jdbc.execute("CREATE INDEX IF NOT EXISTS " + INDEX + " ON " + TABLE
+ " (connection_id, " + COLUMN + " text_pattern_ops)");
} catch (RuntimeException e) {
log.warn("Could not create {}: {}", INDEX, e.getMessage());
}

return new Object();
}

/**
* True when {@code normalized_match} exists but was generated by a different
* expression than {@link #NORMALIZE_EXPR}. Compared on the normalized form Postgres
* stores in {@code pg_get_expr}, with whitespace collapsed, since the server rewrites
* the text it was given (adds casts, reorders parens) and a literal comparison would
* report a difference on every start and rewrite the table each time.
*/
private static boolean columnDefinitionDiffers(JdbcTemplate jdbc) {
try {
String stored = jdbc.query(
"SELECT pg_get_expr(d.adbin, d.adrelid) "
+ "FROM pg_attrdef d "
+ "JOIN pg_attribute a ON a.attrelid = d.adrelid AND a.attnum = d.adnum "
+ "WHERE d.adrelid = ?::regclass AND a.attname = ?",
rs -> rs.next() ? rs.getString(1) : null, TABLE, COLUMN);
if (stored == null) {
return false; // column not present yet — nothing stale to drop
}
return !squash(stored).contains("btrim");
} catch (RuntimeException e) {
log.warn("Could not inspect {}.{} definition: {}", TABLE, COLUMN, e.getMessage());
return false;
}
}

private static String squash(String s) {
return s.replaceAll("\\s+", "").toLowerCase();
}

private static boolean tableExists(JdbcTemplate jdbc, String table) {
try {
Integer found = jdbc.queryForObject(
"SELECT COUNT(*) FROM information_schema.tables "
+ "WHERE table_schema = current_schema() AND table_name = ?",
Integer.class, table);
return found != null && found > 0;
} catch (RuntimeException e) {
log.warn("Could not check for table {}: {}", table, e.getMessage());
return false;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,41 @@ public ResponseEntity<List<SlowQueryAnalyticsService.CustomerSummary>> listCusto
return ResponseEntity.ok(analyticsService.listCustomers(connectionId));
}

/** Every slow query attributed to one customer, ranked by their mean exec time. */
/**
* Every slow query attributed to one customer, ranked by their mean exec time.
*
* <p>The customer id is a <b>query parameter</b>, not a path segment, because it is a
* literal value read out of the tenant column — application data, which can contain
* {@code /}, {@code ?} or {@code #}. Such an id is unreachable as a path segment under
* any encoding: raw, the slash splits the path; percent-encoded, Jetty rejects it with
* {@code 400 Ambiguous URI path separator}. Both were reproduced against this backend
* with the tenant value {@code acct/77?x=1}, whose rows were simply invisible in the
* By-Customer view.
*/
@GetMapping("/{connectionId}/customer-queries")
public ResponseEntity<List<SlowQueryAnalyticsService.CustomerQueryRow>> customerQueries(
@PathVariable String connectionId,
@RequestParam String customerId) {
accessControlService.assertCanReadConnectionContent(connectionId);
return ResponseEntity.ok(analyticsService.queriesForCustomer(connectionId, customerId));
}

/** Literal-bearing samples for one (customer, query) pair — copyable SQL. */
@GetMapping("/{connectionId}/customer-query-samples")
public ResponseEntity<List<SlowQueryAnalyticsService.QuerySample>> customerQuerySamples(
@PathVariable String connectionId,
@RequestParam String customerId,
@RequestParam String fingerprint) {
accessControlService.assertCanReadConnectionContent(connectionId);
return ResponseEntity.ok(
analyticsService.samplesForCustomerQuery(connectionId, customerId, fingerprint));
}

/**
* @deprecated superseded by {@link #customerQueries}; a customer id containing a
* slash cannot be expressed here. Retained so existing clients keep working.
*/
@Deprecated
@GetMapping("/{connectionId}/customer/{customerId}/queries")
public ResponseEntity<List<SlowQueryAnalyticsService.CustomerQueryRow>> queriesForCustomer(
@PathVariable String connectionId,
Expand All @@ -87,7 +121,10 @@ public ResponseEntity<List<SlowQueryAnalyticsService.CustomerQueryRow>> queriesF
return ResponseEntity.ok(analyticsService.queriesForCustomer(connectionId, customerId));
}

/** Literal-bearing samples for one (customer, query) pair — copyable SQL. */
/**
* @deprecated superseded by {@link #customerQuerySamples}; see above.
*/
@Deprecated
@GetMapping("/{connectionId}/customer/{customerId}/query/{fingerprint}/samples")
public ResponseEntity<List<SlowQueryAnalyticsService.QuerySample>> samplesForCustomerQuery(
@PathVariable String connectionId,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,12 @@
@Slf4j
public class WorkloadAnalysisController {

// Reads use canReadContent; only `run` requires canManageContent. The reads were all
// gated on manage, which is the write tier — EffectiveConnectionAccess's own comment
// lists slow-query analytics under read. Latent today because every grant resolves to
// FULL_CONTENT (so both predicates are true), but it would deny the whole Workload tab
// to a read-only grant the moment one is reintroduced.

private final WorkloadAnalysisService workloadAnalysisService;
private final WorkloadAnalysisReportRepository reportRepository;
private final AccessControlService accessControlService;
Expand Down Expand Up @@ -64,7 +70,7 @@ public ResponseEntity<Map<String, Object>> run(@PathVariable String connectionId
/** Lightweight poll payload — status + progress without the full report blob. */
@GetMapping("/{connectionId}/status")
public ResponseEntity<Map<String, Object>> status(@PathVariable String connectionId) {
accessControlService.assertCanManageConnectionContent(connectionId);
accessControlService.assertCanReadConnectionContent(connectionId);
return reportRepository.findFirstByConnectionIdOrderByStartedAtDesc(connectionId)
.map(r -> ResponseEntity.ok(Map.<String, Object>of(
"reportId", r.getId(),
Expand All @@ -81,7 +87,7 @@ public ResponseEntity<Map<String, Object>> status(@PathVariable String connectio
/** The full latest report (with the composed sections). 204 if none yet. */
@GetMapping("/{connectionId}/latest")
public ResponseEntity<WorkloadAnalysisReport> latest(@PathVariable String connectionId) {
accessControlService.assertCanManageConnectionContent(connectionId);
accessControlService.assertCanReadConnectionContent(connectionId);
return reportRepository.findFirstByConnectionIdOrderByStartedAtDesc(connectionId)
.map(ResponseEntity::ok)
.orElseGet(() -> ResponseEntity.noContent().build());
Expand All @@ -92,7 +98,7 @@ public ResponseEntity<WorkloadAnalysisReport> latest(@PathVariable String connec
public ResponseEntity<WorkloadAnalysisReport> getReport(@PathVariable String reportId) {
return reportRepository.findById(reportId)
.map(r -> {
accessControlService.assertCanManageConnectionContent(r.getConnectionId());
accessControlService.assertCanReadConnectionContent(r.getConnectionId());
return ResponseEntity.ok(r);
})
.orElseGet(() -> ResponseEntity.notFound().build());
Expand All @@ -101,7 +107,7 @@ public ResponseEntity<WorkloadAnalysisReport> getReport(@PathVariable String rep
/** Recent report history (newest first), metadata only via the entity. */
@GetMapping("/{connectionId}/history")
public ResponseEntity<List<WorkloadAnalysisReport>> history(@PathVariable String connectionId) {
accessControlService.assertCanManageConnectionContent(connectionId);
accessControlService.assertCanReadConnectionContent(connectionId);
return ResponseEntity.ok(reportRepository.findTop20ByConnectionIdOrderByStartedAtDesc(connectionId));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,10 @@
import com.dbaagent.model.QueryLineage;
import org.springframework.data.jpa.repository.JpaRepository;
import org.springframework.data.jpa.repository.Query;
import org.springframework.data.jpa.repository.Modifying;
import org.springframework.data.domain.Pageable;
import org.springframework.data.repository.query.Param;
import org.springframework.transaction.annotation.Transactional;
import org.springframework.stereotype.Repository;

import java.time.LocalDateTime;
Expand Down Expand Up @@ -147,23 +149,32 @@ QueryLineage findLongestByConnectionIdAndCollapsedQueryTextPrefix(
* Callers must apply the SAME transformation to their prefix on the
* Java side (see {@code SlowQueryAnalyticsService.normalizeForMatching}).
*
* <p>The regex chain runs once per candidate row, so this is intended for
* single-row lookups (LIMIT 1) over the per-connection slice. For
* d840f866-style connections (~5K lineage rows) it returns in a few ms;
* larger connections may benefit from a functional index on the
* normalized expression, but none is needed yet.
* <p>The normalization is <b>precomputed</b> into the stored generated column
* {@code normalized_match} ({@code QueryLineageMatchIndexInitializer}) and matched
* against that, so the regex chain runs once at write time rather than once per
* candidate row on every read. Inlining the expression here made the predicate
* unindexable: Postgres had to materialize a rewritten copy of the whole
* per-connection slice, which measured 36 ms at 1,093 rows and 1,112 ms at 34,976 —
* and {@code recoverFullText} issues this up to 20 times per "view full query" click,
* against a table no retention job prunes. On the same scaled table with the ~120-char
* prefix the caller actually sends, the indexed form plans as an Index Scan at
* 0.428 ms.
*
* <p>Matched against the column <b>directly</b>, with no {@code COALESCE} fallback to
* the inline expression. That fallback is the obvious way to stay safe on a database
* without the column, and it silently undoes the whole fix: wrapping the column in
* {@code COALESCE(...)} makes the predicate non-indexable again. Measured on the same
* 37,504-row table — {@code COALESCE(normalized_match, …)} plans a Seq Scan at
* 48.7 ms, the bare column an Index Scan at 0.39 ms. If the column is ever absent,
* {@code QueryLineageMatchIndexInitializer} logs it at WARN and
* {@code SlowQueryAnalyticsService.recoverFullText} already treats a failed lookup as
* "no longer text available" and returns the sample unchanged.
*/
@Query(value = """
SELECT * FROM query_lineage
WHERE connection_id = :connectionId
AND LENGTH(query_text) > :minLength
AND LOWER(
regexp_replace(
regexp_replace(
REPLACE(query_text, '`', ''),
'\\s*([.,();])\\s*', '\\1', 'g'),
'\\s+', ' ', 'g')
) LIKE :escapedPrefix ESCAPE '\\'
AND normalized_match LIKE :escapedPrefix ESCAPE '\\'
ORDER BY LENGTH(query_text) DESC
LIMIT 1
""", nativeQuery = true)
Expand All @@ -172,4 +183,24 @@ QueryLineage findLongestByConnectionIdAndNormalizedQueryTextPrefix(
@Param("escapedPrefix") String escapedPrefix,
@Param("minLength") int minLength
);

/**
* Drop lineage rows older than the connection's retention window.
*
* <p>This table was never pruned: {@code SlowQueryRetentionService} covered
* {@code slow_query_run}, {@code slow_query_customer_day} and
* {@code slow_query_sample} but not lineage, so it grew without bound while the
* 30-day analytics tables stayed small. That is what made sample recovery degrade
* with age rather than load — the scanned table kept growing even on an idle install.
*
* <p>Keyed on {@code created_at}, which is non-null and already indexed
* ({@code idx_query_lineage_created}).
*/
@Modifying
@Transactional
@Query("DELETE FROM QueryLineage q WHERE q.connectionId = :connectionId "
+ "AND q.createdAt < :cutoff")
int deleteByConnectionIdAndCreatedAtBefore(
@Param("connectionId") String connectionId,
@Param("cutoff") java.time.LocalDateTime cutoff);
}
Loading
Loading