mirror of
https://github.com/open-metadata/OpenMetadata
synced 2026-05-24 09:39:11 +00:00
17c3b8b9b2
* 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>
14 lines
823 B
SQL
14 lines
823 B
SQL
-- 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;
|