Backport to 1.13: reclaim orphaned ingestion pipelines (#29836) + Data Retention build fix (#29984), realign DataRetention with main#30030
Conversation
#28384) * feat(dataRetention): sweep orphan time-series rows for per-type tables Adds `cleanOrphanedTimeSeriesRows()` to `DataRetention` alongside the existing orphan-relationship and orphan-tag sweeps. Each of the five affected DAOs gets a `deleteOrphanedRecords(int limit)` query (MySQL + PostgreSQL) that left-joins to its parent and deletes rows the parent no longer covers: - `testCaseResolutionStatus`: parent link via `entity_relationship` PARENT_OF - `agentExecution`: `agentId` → `ai_application_entity.id` - `mcpExecution`: `serverId` → `mcp_server_entity.id` - `profile_data`: `entityFQNHash` → `table_entity.fqnHash` - `query_cost_time_series`: `entityFQNHash` → `query_entity.fqnHash` The sweep runs after `cleanOrphanedRelationshipsAndHierarchies()` so the PARENT_OF check sees the post-cleanup `entity_relationship` state. Pairs with PR #28367, where the bulk hard-delete cascade now skips time-series children and relies on `DataRetention` to reclaim them out-of-band. Adds `OrphanedTimeSeriesCleanupIT` covering all five per-type queries: inserts a real-parent row and a bogus-parent row through the existing DAO `insert(...)` paths, runs `deleteOrphanedRecords(BATCH)`, asserts the orphan is gone and the valid row is preserved. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * test(dataRetention): fix orphan-cleanup IT name lengths and McpExecution table arg - McpExecutionDAO.insertWithoutExtension uses `<table>` placeholder; the test was passing `null`, which made Jdbi fail to render the SQL template. Pass the literal table name `mcp_execution_entity`. - `ns.prefix(...)` embeds class + method names, so chaining it through database -> schema -> table -> auto-created test_suite pushed the test_suite `name` column past its VARCHAR(256) bound. Use `ns.uniqueShortId()` for the hierarchy components and shorten the test method names so the resulting FQN stays well under the column limit. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * review(dataRetention): bound profile-data orphan delete by rows + clarify PARENT_OF=9 Addresses two PR review findings: - `profiler_data_time_series.deleteOrphanedRecords` previously LIMITed distinct `entityFQNHash` values, then deleted every row for each hash — a batch could delete tens of millions of rows if many orphan hashes each had thousands of rows. Switch to row-level limiting: single-table `DELETE ... WHERE NOT EXISTS (...) LIMIT N` on MySQL, and `ctid IN (SELECT ... LIMIT N)` on PostgreSQL (the table has no `id` column, so we use Postgres ctid for the inner subquery). This matches the row-count cap used by the other four orphan-cleanup queries. - Annotate `er.relation = 9` in the testCaseResolutionStatus query with a `// 9 = Relationship.PARENT_OF` inline comment plus a leading block comment noting the ordinal is stable because the enum appends new values. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> (cherry picked from commit 5f5a123)
…null testDefinition during reindex (#29836) Two reindex failures observed in SearchIndexingApplication that DataRetention did not cover: 1. Orphaned ingestion pipelines. When a service (or test suite) is hard-deleted without the cascade reaching its ingestion pipeline, the pipeline row survives with no incoming CONTAINS relationship. Reindex then throws "Entity type ingestionPipeline <id> does not have expected relationship contains to/from entity type null" because IngestionPipelineRepository.setFields resolves the container via getContainer (mustHaveRelationship=true). Nothing swept these. Adds OrphanIngestionPipelineCleanup (mirrors OrphanTestCaseRelationshipCleanup) wired into DataRetention: pages pipelines, finds ones with no container, and hard-deletes them via the standard delete with a force-delete fallback (search doc, pipeline-status time series, relationship rows, entity row) since the standard delete itself resolves the missing container and throws. 2. NullPointerException building the TestCase search doc. TestCaseIndex called testCase.getTestDefinition().getId() with no null check, so a test case whose testDefinition relationship is gone NPE'd. Unlike the "expected relationship" errors, this NPE is not classified as a stale-reference warning, so it failed the reindex outright. Guards the lookup exactly like TestCaseResultIndex does. Adds OrphanIngestionPipelineCleanupIT covering selective deletion (orphan removed, healthy pipeline preserved). Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> (cherry picked from commit b8d9f3e)
…ckTrace (#29965) * Fixes #20753: replace non-schema jobStackTrace with IndexingError.stackTrace App failure contexts wrote an ad-hoc "jobStackTrace" map key that is not part of the schema. The failureContext.failure field is already typed as IndexingError, which has a schema-defined stackTrace field. Use the typed IndexingError (matching the canonical pattern used across the other apps) instead of raw maps, deduplicating DataRetention's repeated failure blocks. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Convert DataContractValidationApp failure to typed IndexingError Replace the generic Map<String, Object> failure collector with the typed IndexingError: the top-level exception populates message/stackTrace, and the per-entity errors go into the schema's failedEntities (EntityError) list instead of being keyed by FQN in a raw map. Makes all three apps consistent with the schema-defined failureContext.failure shape. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> (cherry picked from commit a33d41c)
* Fix Data Retention Build Issue * Checkstyle (cherry picked from commit ee2a438)
❌ PR checklist incompleteThis PR cannot be merged until the following are addressed on its linked issue:
The fields live on the linked issue in the Shipping project (open the issue → right sidebar → Projects). After you set them, re-run this check (or push a commit) — issue/project changes do not re-trigger it automatically. Maintainers can bypass this check by adding the |
github-actions
Bot
added
backend
safe to test
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| + " WHERE qe.fqnHash IS NULL " | ||
| + " LIMIT :limit" | ||
| + " ) sub" | ||
| + ")", |
There was a problem hiding this comment.
When query_cost_time_series.entityFQNHash is NULL, the left join cannot match any query_entity row, so this cleanup treats the record as orphaned and DataRetention can hard-delete it. If existing query-cost rows were written without a query FQN hash, the scheduled cleanup removes live cost history instead of only rows whose parent query was deleted.
![gitar-bot[bot]](https://avatars.githubusercontent.com/in/827041?s=60&v=4){kind=small}
| @ConnectionAwareSqlUpdate( | ||
| value = | ||
| "DELETE FROM profiler_data_time_series " | ||
| + "WHERE NOT EXISTS (" | ||
| + " SELECT 1 FROM table_entity te " | ||
| + " WHERE te.fqnHash = profiler_data_time_series.entityFQNHash" | ||
| + ") " | ||
| + "LIMIT :limit", | ||
| connectionType = MYSQL) | ||
| @ConnectionAwareSqlUpdate( | ||
| value = | ||
| "DELETE FROM profiler_data_time_series " | ||
| + "WHERE ctid IN (" | ||
| + " SELECT pdts.ctid FROM profiler_data_time_series pdts " | ||
| + " WHERE NOT EXISTS (" |
There was a problem hiding this comment.
🚨 Bug: Orphan profiler cleanup deletes all column-profile rows
ProfilerDataTimeSeriesDAO.deleteOrphanedRecords deletes any profiler_data_time_series row whose entityFQNHash is absent from table_entity.fqnHash. But column profiles are stored under the COLUMN's FQN (TableRepository.java:1082 passes column.getFullyQualifiedName()), e.g. service.db.schema.table.columnName, whose hash never matches a table_entity.fqnHash (which only holds table FQN hashes). As a result every valid table.columnProfile row is treated as an orphan and hard-deleted on each DataRetention run — silent, permanent loss of all column profiling history. The IT (OrphanedTimeSeriesCleanupIT.profilerDataOrphans) only exercises a table-level table.tableProfile row, so it passes while masking the bug. Fix: the orphan test must resolve the parent TABLE, not require the row's own FQN to be a table. Since the stored value is a hash, prefix matching isn't possible; scope the delete to table-keyed extensions only (e.g. AND extension IN ('table.tableProfile','table.systemProfile')) or add a table-existence check that works for column FQNs before shipping this query.
Restrict the delete to table-keyed extensions so column-profile rows (keyed by column FQN) are never matched. Apply the same extension filter to the Postgres variant. Note: this leaves genuinely orphaned column profiles behind — a complete fix needs a table-existence check that handles column FQNs.:
// MySQL — only sweep table-keyed profiler rows; column profiles are keyed by the
// COLUMN FQN and would otherwise all be flagged as orphans.
"DELETE FROM profiler_data_time_series "
+ "WHERE extension IN ('table.tableProfile','table.systemProfile') "
+ "AND NOT EXISTS ("
+ " SELECT 1 FROM table_entity te "
+ " WHERE te.fqnHash = profiler_data_time_series.entityFQNHash"
+ ") "
+ "LIMIT :limit"
- Apply fix
Check the box to apply the fix or reply for a change | Was this helpful? React with 👍 / 👎
Code Review 🚫 Blocked 0 resolved / 1 findingsBackports ingestion pipeline fixes and aligns DataRetention with main, but contains a critical flaw in ProfilerDataTimeSeriesDAO where orphan cleanup deletes all column-profile rows. 🚨 Bug: Orphan profiler cleanup deletes all column-profile rows📄 openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/CollectionDAO.java:8672-8686 📄 openmetadata-integration-tests/src/test/java/org/openmetadata/it/tests/OrphanedTimeSeriesCleanupIT.java:184-198 ProfilerDataTimeSeriesDAO.deleteOrphanedRecords deletes any profiler_data_time_series row whose entityFQNHash is absent from table_entity.fqnHash. But column profiles are stored under the COLUMN's FQN (TableRepository.java:1082 passes column.getFullyQualifiedName()), e.g. service.db.schema.table.columnName, whose hash never matches a table_entity.fqnHash (which only holds table FQN hashes). As a result every valid Restrict the delete to table-keyed extensions so column-profile rows (keyed by column FQN) are never matched. Apply the same extension filter to the Postgres variant. Note: this leaves genuinely orphaned column profiles behind — a complete fix needs a table-existence check that handles column FQNs.🤖 Prompt for agentsOptionsDisplay: compact → Showing less information. Comment with these commands to change the behavior for this request:
Was this helpful? React with 👍 / 👎 | Gitar |
|
The Python checkstyle failed. Please run You can install the pre-commit hooks with |
|
2 participants
Backports the orphaned-ingestion-pipeline reindex fix to 1.13, and realigns
DataRetention.javaso it is byte-identical tomain.Commits (cherry-picked with
-x, in main's order)feat(dataRetention): sweep orphan time-series rows for per-type tablesdeleteOrphanedRecordstoCollectionDAO; main'sDataRetentioncalls it and 1.13 didn't have it.fix(search-indexing): reclaim orphaned ingestion pipelines and guard null testDefinition during reindexFixes #20753: replace non-schema jobStackTrace with IndexingError.stackTracefailureDetailsfrom rawMapto the schema-typedIndexingError.Fix Data Retention Build Issue#29836 and #29984 cannot compile on 1.13 without #28384 and #29965, so all four are picked together.
Note on the
executeOrphanCleanupconflict1.13 previously received a partial backport of #28159 (the test-case cleanup parts) which silently dropped its
MAX_ORPHAN_CLEANUP_ITERATIONSguard — that guard protectsexecuteOrphanCleanup, which only exists once #28384 lands.Without the cap,
executeOrphanCleanupis a barewhile (true)that spins forever if a delete query keeps reporting non-zero deletions (e.g. rows blocked by an FK constraint), wedging the entire DataRetention job. Resolving that method to main's shape folds the cap back in, so 1.13 does not ship the uncapped loop.Result
DataRetention.java,OrphanIngestionPipelineCleanup.java,TestCaseIndex.java,OmAppJobListener.javaandDataContractValidationApp.javaare now identical tomain.Verification
openmetadata-servicecompiles (main + test sources)openmetadata-integration-testscompiles, including the two new ITs (OrphanIngestionPipelineCleanupIT,OrphanedTimeSeriesCleanupIT)mvn spotless:checkcleanOmAppJobListenerTest— 13/13 passjobStackTrace/HashMapfailure-context residue left inDataRetention🤖 Generated with Claude Code
Greptile Summary
This PR backports DataRetention cleanup and app failure-context fixes. The main changes are:
IndexingErrorrecords.Confidence Score: 5/5
This looks safe to merge after checking the query-cost cleanup edge case.
openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/CollectionDAO.java
Important Files Changed
IndexingErrorwith per-entity failures.FailureContext.failureasIndexingError.Reviews (1): Last reviewed commit: "Fix Data Retention Build Issue (#29984)" | Re-trigger Greptile
Context used: