fix(migration): preempt PDTS duplicates and recover invalid index in 1.12.9 (#28238)
* fix(migration): preempt PDTS duplicates and recover invalid index in 1.12.9
CREATE UNIQUE INDEX CONCURRENTLY aborts when it hits existing duplicate
keys but leaves an invalid index behind. On migration retry, IF NOT EXISTS
no-ops successfully and gets checksum-logged, after which ADD CONSTRAINT
USING INDEX fails permanently with "index ... is not valid". Hit at a
customer with two duplicate table.systemProfile rows on a 10M-row PDTS.
Adds two idempotent statements before the existing constraint build:
- DO block: drops the invalid index and clears its migration-log entry
when indisvalid=false. No-op on fresh DBs and on already-migrated
environments (where the index is valid and owned by the constraint).
- DELETE: collapses duplicate rows via single hash aggregate on the
4-column key + targeted self-join. Reads only key columns (no json
scan), only touches rows in actual duplicate groups, no-op on clean DBs.
Efficient on multi-million-row PDTS tables.
Existing CREATE INDEX / ALTER TABLE / ANALYZE statements byte-identical
so checksum-matched skips for already-migrated environments still apply.
* fix(migration): self-heal in one pass + schema-scoped invalid-index probe
Addresses Copilot review on #28238:
1. One-pass self-heal. MigrationFile.parseSQLFiles filters already-logged
statements at parse time (MigrationFile.java:83). Clearing the CREATE
log entry from inside a DO block doesn't bring CREATE back into the
current pass's execution list — it would only re-run on the next
migration cycle, leaving the same-pass ALTER to fail again. Replace
the "DROP + clear log" pattern with "DROP + rebuild inline" so a
valid index exists before ALTER runs in the same pass.
Inline rebuild uses non-concurrent CREATE UNIQUE INDEX, which takes a
brief ACCESS EXCLUSIVE lock on the table. Acceptable because this
path fires only when the environment is already in a degraded state.
Normal-path customers go through the CONCURRENTLY build below.
2. Schema-scoped invalid-index probe. pg_class.relname is not
schema-unique. Anchor the lookup via
i.indrelid = 'profiler_data_time_series'::regclass and DROP by index
OID (invalid_idx::regclass), so an invalid index with the same name
in another schema cannot accidentally trigger this branch.
Existing CREATE INDEX / ALTER TABLE / ANALYZE statements byte-identical
to before this PR, so checksum-matched skips still apply for
already-migrated environments. Test gap (Copilot's third comment) for
the recovery scenario tracked as follow-up — existing migration tests
in MigrationWorkflowReprocessingTest are mock-based; verifying recovery
end-to-end needs Postgres integration infrastructure.
* chore(migration): trim verbose comments in 1.12.9/postgres/schemaChanges.sql
Statement bodies unchanged — checksums identical. Detailed mechanism
write-ups live in the commit log / PR description; the file keeps just
the load-bearing intent comment above each statement.
* fix(migration): scope PDTS dedup to operation IS NOT NULL
Addresses Copilot review on PR #28238 discussion r3264066840.
Postgres UNIQUE treats NULLs as DISTINCT by default, so the constraint
on (entityFQNHash, extension, operation, timestamp) permits multiple
rows where operation IS NULL — i.e. table.tableProfile and
table.columnProfile rows that share the other key columns.
The previous dedup used GROUP BY which treats NULLs as equal, so it
would have collapsed retry-induced tableProfile / columnProfile pairs
that the restored constraint never actually blocked. Restricting the
subquery to operation IS NOT NULL (plus a defensive entityFQNHash IS
NOT NULL) aligns dedup with constraint semantics.
DMG's customer rows were all table.systemProfile (operation = INSERT),
so this still removes the customer dupes correctly. tableProfile /
columnProfile retry duplicates — if they exist — stay as-is, which is
the same outcome the unique constraint would produce on its own.
* perf(migration): boost work_mem / maintenance_work_mem for PDTS dedup at scale
Mirrors the tuning pattern from 1.9.9/postgres/postDataMigrationSQLScript.sql
(same table, same operation class). On 50M-row PDTS the dedup DELETE's hash
aggregate spills to disk with default work_mem=4MB, adding ~30-60s of disk
I/O. Bumping work_mem to 256MB keeps the aggregate in memory;
maintenance_work_mem=512MB lets CREATE UNIQUE INDEX CONCURRENTLY sort in
memory too.
Session-level (not SET LOCAL) because schemaChanges runs in autocommit
(CREATE INDEX CONCURRENTLY requires it) — SET LOCAL would reset between
statements. RESET at the end of the file restores defaults before the
connection returns to the Hikari pool.
Expected runtime impact at customer scale:
20M rows: ~30s tuned vs ~40s default
50M rows: ~40s tuned vs ~90s default (avoids spill)
* chore(migration): trim comments on PDTS dedup additions
* chore(migration): drop 1.9.9 reference from mem comment
2026-05-20 12:57:33 +00:00
|
|
|
-- Boost memory for the dedup + index build. RESET at end.
|
|
|
|
|
SET work_mem = '256MB';
|
|
|
|
|
SET maintenance_work_mem = '512MB';
|
|
|
|
|
|
|
|
|
|
-- Dedup before the unique index rebuild. NULL filter on operation: Postgres
|
|
|
|
|
-- UNIQUE treats NULLs as DISTINCT, so the constraint never blocked tableProfile
|
|
|
|
|
-- / columnProfile rows (operation = NULL). GROUP BY treats NULLs as equal —
|
|
|
|
|
-- without the filter we'd collapse rows the constraint never rejected.
|
|
|
|
|
DELETE FROM profiler_data_time_series p
|
|
|
|
|
USING (
|
|
|
|
|
SELECT entityFQNHash, extension, operation, "timestamp", MAX(ctid) AS keep_ctid
|
|
|
|
|
FROM profiler_data_time_series
|
|
|
|
|
WHERE operation IS NOT NULL
|
|
|
|
|
AND entityFQNHash IS NOT NULL
|
|
|
|
|
GROUP BY entityFQNHash, extension, operation, "timestamp"
|
|
|
|
|
HAVING COUNT(*) > 1
|
|
|
|
|
) d
|
|
|
|
|
WHERE p.entityFQNHash = d.entityFQNHash
|
|
|
|
|
AND p.extension = d.extension
|
|
|
|
|
AND p.operation = d.operation
|
|
|
|
|
AND p."timestamp" = d."timestamp"
|
|
|
|
|
AND p.ctid <> d.keep_ctid;
|
|
|
|
|
|
|
|
|
|
-- Recover from a prior failed CREATE UNIQUE INDEX CONCURRENTLY: drop the
|
|
|
|
|
-- invalid leftover and rebuild inline so ALTER below can promote it.
|
|
|
|
|
DO $$
|
|
|
|
|
DECLARE
|
|
|
|
|
invalid_idx oid;
|
|
|
|
|
BEGIN
|
|
|
|
|
SELECT i.indexrelid INTO invalid_idx
|
|
|
|
|
FROM pg_index i
|
|
|
|
|
JOIN pg_class idx ON idx.oid = i.indexrelid
|
|
|
|
|
WHERE idx.relname = 'profiler_data_time_series_unique_hash_extension_ts'
|
|
|
|
|
AND i.indrelid = 'profiler_data_time_series'::regclass
|
|
|
|
|
AND NOT i.indisvalid;
|
|
|
|
|
|
|
|
|
|
IF invalid_idx IS NOT NULL THEN
|
|
|
|
|
EXECUTE 'DROP INDEX ' || invalid_idx::regclass;
|
|
|
|
|
EXECUTE 'CREATE UNIQUE INDEX profiler_data_time_series_unique_hash_extension_ts '
|
|
|
|
|
|| 'ON profiler_data_time_series '
|
|
|
|
|
|| '(entityFQNHash, extension, operation, "timestamp")';
|
|
|
|
|
END IF;
|
|
|
|
|
END $$;
|
|
|
|
|
|
fix(profiler): N+1 / missing-index regression on /tables/.../columns?fields=profile (#3488) (#27746)
* fix(profiler): N+1 / missing-index regression on /tables/.../columns?fields=profile (#3488)
Root cause
----------
The 1.9.9 migration introduced two separate index regressions on
`profiler_data_time_series`:
1. **PostgreSQL**: `schemaChanges.sql` explicitly dropped the unique
constraint `profiler_data_time_series_unique_hash_extension_ts`
(entityFQNHash, extension, operation, timestamp) to allow altering the
generated `operation` column expression, but never recreated it. After
the migration the table kept only the `(extension, timestamp)` index,
which is useless for queries filtering by `entityFQNHash`.
2. **MySQL/both**: `postDataMigrationSQLScript.sql` created temporary indexes
(idx_pdts_entityFQNHash, idx_pdts_composite, etc.) for its bulk UPDATE
pass and then dropped **all** of them, including the only index covering
`entityFQNHash`.
The batch query issued by `getLatestExtensionsBatch()` when
`fields=profile` is requested:
SELECT entityFQNHash, MAX(timestamp) FROM profiler_data_time_series
WHERE entityFQNHash IN (...N hashes...) AND extension = 'table.columnProfile'
GROUP BY entityFQNHash
required an `(entityFQNHash, extension, timestamp)` index. Without it the
database performs a full table scan. On production deployments with
millions of profiler rows this caused 100+ second response times (Grafana:
106 770 ms; 99 % in DB; 93 dbOps). Without `profile` in the fields param
the same endpoint returned in ~150-220 ms.
A secondary N+1 bug existed independently of the index: `customMetrics`
in fields called `getCustomMetrics(table, column)` once per paginated
column, issuing up to N identical queries against `entity_extension` and
then filtering in Java.
Fix
---
* **migration 2.0.2** (MySQL + PostgreSQL): `CREATE INDEX IF NOT EXISTS
idx_pdts_fqnhash_ext_ts ON profiler_data_time_series(entityFQNHash,
extension, timestamp)`. The `IF NOT EXISTS` guard makes the migration
safe to re-run and handles both upgrade and fresh-install paths.
* **`getTableColumnsInternal`** — `customMetrics` block: fetch all column
custom metrics for the table in one query, group by column name in Java,
then distribute. Reduces N queries to 1.
* **`getTableColumnsInternal`** — `profile` block: skip the duplicate
`populateEntityFieldTags` call when `tags` was already fetched earlier in
the same request, saving one prefix-scan on `tag_usage` per request.
Related: PR #26855 (fixed N+1 tag queries on the list-tables path but left
the profiler-index and customMetrics N+1 untouched on the columns sub-path).
* fix(profiler): restore unique constraint on profiler_data_time_series + batch column extension/customMetrics fetch
Move the migration from 2.0.2/ to 1.12.8/ and switch from a non-unique
covering index to restoring the original unique constraint dropped in
1.9.9. The two-phase CREATE UNIQUE INDEX CONCURRENTLY + ADD CONSTRAINT
USING INDEX pattern avoids the ACCESS EXCLUSIVE lock on the hot
profiler_data_time_series table during the upgrade. Closes the 1.9.9
regression and brings Postgres back in line with MySQL (which never lost
the constraint). The leading (entityFQNHash, extension) prefix serves
the column-profile batch query — same shape MySQL has been running
without 504s. MySQL needs no migration.
Java side, eliminates two more N+1 patterns that compound the latency at
customer scale:
* getTableColumnsInternal extension block: replaced per-column
getColumnExtension() loop with a single getExtensionsByJsonSchema()
call, grouped by column FQN-hash in Java.
* searchTableColumnsInternal customMetrics block: applied the same
batch-fetch pattern already used in getTableColumnsInternal, replacing
per-column getCustomMetrics() with one getExtensions() call.
New DAO method on EntityExtensionDAO:
getExtensionsByJsonSchema(id, jsonSchema) — selects extensions for a
table id filtered by the jsonschema discriminator. Required because
column extensions are stored with MD5-hashed extension keys and have
no shared prefix the existing getExtensions(id, prefix) could use.
* chore(profiler): address review feedback — empty-list literal + accurate test comments
* Replace `new ArrayList<>()` default in `metricsByColumn.getOrDefault(...)`
with `List.of()` at both call sites in `TableRepository` (getTableColumnsInternal
and searchTableColumnsInternal). `getOrDefault` evaluates its default eagerly,
so the new ArrayList allocates per-column even when the key is found —
unnecessary work on a hot path.
* Reword two stale test comments in `test_getColumnsWithProfileField_correctnessAndNoBatchRegression`:
- "all four field combinations" → "the three field combinations exercised below"
- "(c) duplicate populateEntityFieldTags must not run twice" → describe the
observable contract the assertions actually verify (tags + profile both
present), not the internal call count.
* fix(profiler): force outer index scan in getLatestExtensionsBatch by pushing IN list to the join
The getLatestExtensionsBatch query was the right shape for correctness but
the planner — on Postgres at customer scale, with the new unique constraint
in place — was still choosing a parallel sequential scan over the full
profiler_data_time_series table for the outer side of the JOIN, rather than
a merge join with index scan on both sides.
Inner subquery: filtered by `entityFQNHash IN (...)`, used the index.
Outer: only filtered by `p.extension = :extension`, no IN list, planner
couldn't infer the transitive constraint that p.entityFQNHash must equal
one of the inner hashes (because it's enforced through the JOIN ON clause,
not a WHERE predicate). Result: full table scan reading 6.7M+ rows even
when the actual answer is 23 rows.
Adding the redundant `AND p.entityFQNHash IN (<entityFQNHashes>)` to the
outer WHERE makes the constraint explicit. The result set is unchanged
(implied by the join condition), but the planner can now use the unique
index for the outer access too.
Verified on the AUT dump (6.94M-row pdts):
EXPLAIN of the batch query: 7,234ms → 79ms (Hash Join + Parallel Seq
Scan → Merge Join + Index Only Scan).
Live API /columns?fields=profile&include=all: 6-36 seconds → 22-28ms
(warm) / 1.9s (very first call). 250-1000x improvement, depending on
cache state.
Same SQL works on both engines; no @ConnectionAwareSqlQuery split needed.
* test(profiler): shorten classification/tag fixture names in IT to fit varchar(256)
The IT fixture for test_getColumnsWithProfileField_correctnessAndNoBatchRegression
was building a tagFQN of `<classification>.<tag>` where each part went through
TestNamespace.prefix(). With the descriptive method name (62 chars) + class
name (15 chars) + namespace UUID (32 chars) plus the `profile_test_cls` /
`profile_test_tag` base names (16 chars each), the resulting tagFQN was 263
characters — over the tag_usage.tagFQN VARCHAR(256) limit:
ERROR: value too long for type character varying(256)
Shorten the fixture base names from `profile_test_cls`/`profile_test_tag` to
`cls`/`tag`. The namespace prefix already encodes test isolation (class +
method + UUID), so the base name doesn't need to repeat that context.
New tagFQN length: 237 chars (cls__<32>__TableResourceIT__<62>.tag__<32>__TableResourceIT__<62>),
comfortably under 256.
* fix(table): include extensionKey in column-extension deserialize warn log
Addresses gitar-bot review on PR #27746: the warning log on failed column-
extension deserialization only had table.getId(), so operators could not
pinpoint which row was bad. Add record.extensionName() (the entity_extension
row key) to the log. No extra iteration - record is already in scope inside
the catch.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* chore(migration): move profiler unique-constraint migration to 1.12.9
1.12.8 was already published with the PII classification fix from #27910.
Move the profiler_data_time_series unique-constraint restore (this PR's
postgres migration) to 1.12.9 so customers upgrading past the published
1.12.8 still pick it up.
Add a MySQL placeholder schemaChanges.sql for 1.12.9 consistent with the
1.12.7 convention — MySQL was unaffected by the 1.9.9 regression
(MODIFY COLUMN re-evaluates generated expressions in place without
touching the constraint, so MySQL still has the constraint from 1.1.5).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* refactor(table): extract batchFetchCustomMetricsByColumn helper
Addresses PR #27746 Copilot review:
- Dedupe custom-metric batch logic between getTableColumnsInternal and
searchTableColumnsInternal.
- Reword IT inline comment to reflect what the test actually validates
(completes within timeout + correct profiles) instead of claiming it
inspects query plans.
---------
Co-authored-by: Claude <noreply@anthropic.com>
2026-05-14 13:56:39 +00:00
|
|
|
-- Restore the unique constraint dropped in 1.9.9. Closes the 1.9.9 regression that caused
|
|
|
|
|
-- /columns?fields=profile 504s, and brings Postgres back in line with MySQL (which never
|
|
|
|
|
-- lost it). The leading (entityFQNHash, extension) prefix serves the column-profile batch query.
|
|
|
|
|
-- Two-phase: CONCURRENTLY build avoids ACCESS EXCLUSIVE lock; ADD CONSTRAINT USING INDEX
|
|
|
|
|
-- promotes the built index without re-scanning.
|
|
|
|
|
CREATE UNIQUE INDEX CONCURRENTLY IF NOT EXISTS
|
|
|
|
|
profiler_data_time_series_unique_hash_extension_ts
|
|
|
|
|
ON profiler_data_time_series (entityFQNHash, extension, operation, timestamp);
|
|
|
|
|
|
|
|
|
|
ALTER TABLE profiler_data_time_series
|
|
|
|
|
ADD CONSTRAINT profiler_data_time_series_unique_hash_extension_ts
|
|
|
|
|
UNIQUE USING INDEX profiler_data_time_series_unique_hash_extension_ts;
|
|
|
|
|
|
|
|
|
|
ANALYZE profiler_data_time_series;
|
fix(migration): preempt PDTS duplicates and recover invalid index in 1.12.9 (#28238)
* fix(migration): preempt PDTS duplicates and recover invalid index in 1.12.9
CREATE UNIQUE INDEX CONCURRENTLY aborts when it hits existing duplicate
keys but leaves an invalid index behind. On migration retry, IF NOT EXISTS
no-ops successfully and gets checksum-logged, after which ADD CONSTRAINT
USING INDEX fails permanently with "index ... is not valid". Hit at a
customer with two duplicate table.systemProfile rows on a 10M-row PDTS.
Adds two idempotent statements before the existing constraint build:
- DO block: drops the invalid index and clears its migration-log entry
when indisvalid=false. No-op on fresh DBs and on already-migrated
environments (where the index is valid and owned by the constraint).
- DELETE: collapses duplicate rows via single hash aggregate on the
4-column key + targeted self-join. Reads only key columns (no json
scan), only touches rows in actual duplicate groups, no-op on clean DBs.
Efficient on multi-million-row PDTS tables.
Existing CREATE INDEX / ALTER TABLE / ANALYZE statements byte-identical
so checksum-matched skips for already-migrated environments still apply.
* fix(migration): self-heal in one pass + schema-scoped invalid-index probe
Addresses Copilot review on #28238:
1. One-pass self-heal. MigrationFile.parseSQLFiles filters already-logged
statements at parse time (MigrationFile.java:83). Clearing the CREATE
log entry from inside a DO block doesn't bring CREATE back into the
current pass's execution list — it would only re-run on the next
migration cycle, leaving the same-pass ALTER to fail again. Replace
the "DROP + clear log" pattern with "DROP + rebuild inline" so a
valid index exists before ALTER runs in the same pass.
Inline rebuild uses non-concurrent CREATE UNIQUE INDEX, which takes a
brief ACCESS EXCLUSIVE lock on the table. Acceptable because this
path fires only when the environment is already in a degraded state.
Normal-path customers go through the CONCURRENTLY build below.
2. Schema-scoped invalid-index probe. pg_class.relname is not
schema-unique. Anchor the lookup via
i.indrelid = 'profiler_data_time_series'::regclass and DROP by index
OID (invalid_idx::regclass), so an invalid index with the same name
in another schema cannot accidentally trigger this branch.
Existing CREATE INDEX / ALTER TABLE / ANALYZE statements byte-identical
to before this PR, so checksum-matched skips still apply for
already-migrated environments. Test gap (Copilot's third comment) for
the recovery scenario tracked as follow-up — existing migration tests
in MigrationWorkflowReprocessingTest are mock-based; verifying recovery
end-to-end needs Postgres integration infrastructure.
* chore(migration): trim verbose comments in 1.12.9/postgres/schemaChanges.sql
Statement bodies unchanged — checksums identical. Detailed mechanism
write-ups live in the commit log / PR description; the file keeps just
the load-bearing intent comment above each statement.
* fix(migration): scope PDTS dedup to operation IS NOT NULL
Addresses Copilot review on PR #28238 discussion r3264066840.
Postgres UNIQUE treats NULLs as DISTINCT by default, so the constraint
on (entityFQNHash, extension, operation, timestamp) permits multiple
rows where operation IS NULL — i.e. table.tableProfile and
table.columnProfile rows that share the other key columns.
The previous dedup used GROUP BY which treats NULLs as equal, so it
would have collapsed retry-induced tableProfile / columnProfile pairs
that the restored constraint never actually blocked. Restricting the
subquery to operation IS NOT NULL (plus a defensive entityFQNHash IS
NOT NULL) aligns dedup with constraint semantics.
DMG's customer rows were all table.systemProfile (operation = INSERT),
so this still removes the customer dupes correctly. tableProfile /
columnProfile retry duplicates — if they exist — stay as-is, which is
the same outcome the unique constraint would produce on its own.
* perf(migration): boost work_mem / maintenance_work_mem for PDTS dedup at scale
Mirrors the tuning pattern from 1.9.9/postgres/postDataMigrationSQLScript.sql
(same table, same operation class). On 50M-row PDTS the dedup DELETE's hash
aggregate spills to disk with default work_mem=4MB, adding ~30-60s of disk
I/O. Bumping work_mem to 256MB keeps the aggregate in memory;
maintenance_work_mem=512MB lets CREATE UNIQUE INDEX CONCURRENTLY sort in
memory too.
Session-level (not SET LOCAL) because schemaChanges runs in autocommit
(CREATE INDEX CONCURRENTLY requires it) — SET LOCAL would reset between
statements. RESET at the end of the file restores defaults before the
connection returns to the Hikari pool.
Expected runtime impact at customer scale:
20M rows: ~30s tuned vs ~40s default
50M rows: ~40s tuned vs ~90s default (avoids spill)
* chore(migration): trim comments on PDTS dedup additions
* chore(migration): drop 1.9.9 reference from mem comment
2026-05-20 12:57:33 +00:00
|
|
|
|
|
|
|
|
-- Reset session memory before the connection returns to the pool.
|
|
|
|
|
RESET work_mem;
|
|
|
|
|
RESET maintenance_work_mem;
|