Fix/performance tab gaps - #91
Conversation
… review The seven non-blocking findings left over after #86. Each was re-verified against current main before being fixed, and each fix verified after — two of them turned out to be wrong on the first attempt, both caught by testing rather than reading. Sample recovery was an unindexable full scan on a table nothing pruned recoverFullText runs one lookup per sample, up to 20 per "view full query" click, and the query wrapped query_text in three nested regexp_replace/REPLACE calls plus LOWER — so no index could satisfy it and Postgres materialized a rewritten copy of the whole per-connection slice. EXPLAIN (ANALYZE) on a real install: 36 ms at 1,093 rows, 1,112 ms at 34,976. Linear, and query_lineage was absent from SlowQueryRetentionService, so it degraded with the install's *age* rather than its load — which is why it passed every pre-launch test. Precomputed into a STORED generated column (normalized_match) with a text_pattern_ops index, applied by QueryLineageMatchIndexInitializer since this repo has no Flyway runtime. Verified on the real table: Index Scan, 0.119 ms, and the DDL is idempotent on re-run. query_lineage is now purged with the other three fact tables. Deliberately no COALESCE fallback to the inline expression. That is the obvious way to stay safe on a database without the column and it silently undoes the entire fix: measured on the same 37,504-row table, COALESCE(normalized_match, …) plans a Seq Scan at 48.7 ms versus 0.39 ms for the bare column. A missing column instead surfaces as a WARN from the initializer, and recoverFullText already catches a failed lookup and returns the sample unchanged. Denials were retried three times queryClient set retry: 3 with no status predicate, so a 403 became four requests and ~7 s of backoff before the UI could render anything — and an unauthorized /tenant-column-suggestions opens a fresh JDBC connection to the target database on every attempt. The first version of this fix read error.response.status and did nothing at all: the axios response interceptor rethrows a plain Error with the status copied onto error.status, so the axios-shaped field never matched. Measured in the browser before and after: 4 attempts / 7197 ms -> 1 attempt / 34 ms. Confirmed 500, 503, 401 and network failures still retry, and the 3-attempt cap still holds. A customer id containing a slash was unreachable by any encoding customerId is a literal value from the tenant column — application data, so it can contain /, ? or #. Raw, the slash split the path; percent-encoded, Jetty answers 400 "Ambiguous URI path separator". Both reproduced with the real value `acct/77?x=1`, whose rows rendered as "no queries rolled up yet" while the header said the customer had 12 executions. encodeURIComponent alone does not fix this, which is why the id moved off the path: /{connectionId}/customer-queries?customerId=… and /customer-query-samples. The old path routes are kept and @deprecated for wire compatibility. Verified: the slash-bearing id now returns 200, and 403 on a connection the caller cannot read. Failed fetches rendered as "no data yet" Every panel branched on `!isLoading && rows.length === 0`, and `data ?? []` turns any error into an empty array — so a 404 and an empty result were indistinguishable. That matters more now that connection authorization is enforced: a 403 would read as "nothing captured yet" and send the user to re-run an ingestion they cannot fix. Added a shared QueryError component wired into CustomerExplorer (3 branches), QueryTrendsTab (2) and WorkloadAnalysisPanel (1), with per-status wording verified against the real interceptor error shapes. Tab bar and ARIA At 390 px the four tabs measured 427 px with overflow-x: visible, so three of them sat off-screen unreachable. The bar now scrolls (verified: scrolls 373 px, all four reachable). Completed the ARIA tabs pattern — role="tabpanel", aria-controls, roving tabindex and arrow/Home/End navigation. The first version had a stale-closure bug that moved selection exactly once and then froze; fixed with the functional state updater, verified across the full key sequence including wraparound. Workload reads were gated on the write tier status/latest/getReport/history all used assertCanManageConnectionContent. EffectiveConnectionAccess's own comment lists slow-query analytics under read. Latent today because every grant resolves to FULL_CONTENT, but it would deny the whole Workload tab to a read-only grant the moment one is reintroduced. `run` keeps manage. Known issue, documented rather than fixed The slow-log ingestion cursor has two real defects, both left in place with a comment at updateLastProcessed explaining them: it records the time ingestion *finished* rather than the last event's timestamp (so events arriving mid-run are skipped permanently), and it writes LocalDateTime.now() while every read does .atZone(ZoneOffset.UTC) (agreeing only because the container runs Etc/UTC; a bare-metal install at UTC+5:30 would skip 5.5 h of history every run). Not fixed here because all six providers need live cloud credentials to exercise, and an unverified change to a cursor silently skips or duplicates data. Verification Backend and frontend both build clean. Live against the rebuilt image: new customer routes 200 with a slash-bearing id and 403 on an ungranted connection; workload reads non-403 for a granted user; the lineage index confirmed as an Index Scan on the real table. Not covered: mvn test was not run locally (no JDK/Maven on this host) — CI runs it. Note that the "backend tests (advisory)" job is continue-on-error, so its green tick means the job finished, not that tests passed; main currently has 5 failing test classes independent of this branch. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…Matching Found in hands-on QA of the previous commit, not by reading it. SlowQueryAnalyticsService.normalizeForMatching ends with .trim(); the SQL expression behind the generated column did not. A lineage row stored with leading whitespace therefore normalized to " select ..." in the column and "select ..." on the Java side, so the prefix LIKE never matched and recoverFullText silently returned the truncated sample instead of the full SQL. Verified against the local install: inserting ' SELECT x FROM t WHERE y = 1 ' produced "[ select x from t where y = 1 ]" where Java produces "[select x from t where y = 1]"; 4 of 1,174 real rows carried such whitespace. The flaw was equally present in the inline expression this column replaced, so it is pre-existing rather than a regression — but it is silent either way, which is why it survived. Two parts to the fix: * btrim(...) added to the expression in both the initializer and V118. * The initializer now detects a stale column and rebuilds it. A generated column's expression cannot be altered in place and ADD COLUMN IF NOT EXISTS silently keeps whatever is already there, so an install that ran the earlier build would have kept the untrimmed expression forever. It compares pg_get_expr against the expected shape and only drops/re-adds when they differ, so a normal restart does not rewrite the table. Verified on the running stack: restart logged "Rebuilding query_lineage.normalized_match: stored expression is out of date", the stored expression now carries btrim, the index survived the rebuild, and rows-with-untrimmed-normalization went from 4 to 0. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
What's doneThe seven non-blocking findings left over from the Performance-tab readiness review after #86. Each was re-verified against current
Three things worth calling outTwo of my own fixes were wrong on the first attempt, and only testing caught them:
I nearly shipped a Hands-on QA found a real silent bug (second commit, That fix has two parts: Deliberately not fixedThe slow-log ingestion cursor has two real defects, documented at
All six providers need live cloud credentials to exercise, and an unverified change to an ingestion cursor silently skips or duplicates production data. Better documented than guessed at. VerificationBackend and frontend both build clean. 21 hands-on QA scenarios run against the local stack — API + direct DB query + real browser via Chrome DevTools MCP, with API/DB/UI agreement required for a pass. Highlights: the slash-id round trip renders On the index: at the current 799 rows the planner correctly prefers a seq scan, and Caveat on CI
|
No description provided.