feat(ingest/gc): add query cleanup pass for stale SYSTEM queries by aviraj-gour · Pull Request #18120 · datahub-project/datahub

aviraj-gour · 2026-07-01T12:42:42Z

Summary

Adds a QueryCleanup first-pass to the datahub-gc source that soft-deletes old SYSTEM queries by age alone.

Query entities have no first-pass cleanup today, and query is excluded from stateful ingestion's stale-entity removal. As a result, stale SYSTEM queries accumulate as permanent orphans in Elasticsearch and primary storage, causing index bloat and search/UI noise.

Approach

Selection by age alone, no reference gate. Candidates are chosen with a single server-side search filter: source == SYSTEM AND lastModifiedAt < cutoff (where cutoff = now - retention_days). lastModifiedAt is already @Searchable on queryProperties, so no backend change or reindex is needed.
SYSTEM-only. MANUAL queries are never touched; this is enforced in the filter, not a config option.
Soft delete via the workunit stream. For each candidate the source yields a Status(removed=true) workunit — the same mechanism as stateful ingestion's stale-entity removal — and lets the sink batch and write it (the default datahub-rest sink mode is ASYNC_BATCH). The source keeps no concurrency or emit-mode logic for this path.
Hard delete via a worker pool. When hard_delete_entities is set, there is no bulk or async hard-delete endpoint, so the per-URN blocking deletes are parallelized across max_workers threads. Completed futures are drained on the calling thread, so report updates need no locking. (This path skips reference cleanup — prefer the two-pass default.)
Two-pass lifecycle. This pass soft-deletes by default; the existing SoftDeletedEntitiesCleanup already lists query in its default entity types and performs the hard-delete second pass on a later run, additionally calling deleteReferences.
Run-bounding. Both paths stop once limit_entities_delete or runtime_limit_seconds is reached, mirroring SoftDeletedEntitiesCleanup.

Changes

New metadata-ingestion/src/datahub/ingestion/source/gc/query_cleanup.py (QueryCleanupConfig, QueryCleanupReport, QueryCleanup). QueryCleanup.get_workunits() streams candidates and dispatches: dry-run preview, parallel hard delete, or soft-delete workunit emission.
Wire query_cleanup into DataHubGcSource: config field, report base, instantiation, and a guarded stage that yield froms the cleanup workunits, ordered before soft-deleted-entities cleanup.
Add a query_cleanup block to the ingestion-datahub-gc.yaml bootstrap template and bump its version to v7.
Document the pass in metadata-ingestion/docs/sources/datahubgc/.
Unit tests in metadata-ingestion/tests/unit/test_query_cleanup.py.

Config

Disabled by default (enabled: false) because it is destructive. Options: retention_days (30), hard_delete_entities (false — prefer the two-pass default), batch_size (500, search fetch size), max_workers (10, hard-delete parallelism only), limit_entities_delete (25000), runtime_limit_seconds (7200).

Recommended rollout: enable with dry_run: true first and inspect num_queries_found / sample_deleted_queries, then flip to soft-delete with a retention_days at least as large as your largest connector ingestion window (a live query's lastModifiedAt is refreshed on every re-observation, so only queries that have aged out of every ingestion window can cross the cutoff).

Testing

./gradlew :metadata-ingestion:lintFix — clean (ruff + mypy).
pytest tests/unit/test_query_cleanup.py — 10 passing: filter selection, soft-delete workunit emission, parallel hard delete, dry-run preview, deletion/time limits, disabled, and invalid-URN skip (reported as a warning).

Checklist

PR conforms to the Contributing Guideline (PR Title Format)
Tests added
Docs — connector docs + config field descriptions added
Breaking changes — none

Add a QueryCleanup module to datahub-gc that soft-deletes old SYSTEM queries selected by age alone (lastModifiedAt older than retention_days) via a single server-side search filter, with no reference gate. The existing SoftDeletedEntitiesCleanup completes the hard-delete second pass. Query entities previously had no first-pass cleanup and are excluded from stateful ingestion's stale-entity removal, so stale SYSTEM queries accumulated as permanent orphans in Elasticsearch and primary storage. - New query_cleanup.py (config, report, QueryCleanup) with run-bounding (limit_entities_delete, runtime_limit_seconds) and threaded deletes. - Wire query_cleanup into DataHubGcSource, ordered before the soft-deleted-entities cleanup stage. - Add query_cleanup block to the datahub-gc bootstrap template (v7). - Unit tests for filter selection, soft/hard delete, dry-run, and limits. Disabled by default (destructive). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

codecov · 2026-07-01T12:48:26Z

Codecov Report

❌ Patch coverage is 89.24731% with 10 lines in your changes missing coverage. Please review.
✅ All tests successful. No failed tests found.

Files with missing lines	Patch %	Lines
...tion/src/datahub/ingestion/source/gc/datahub_gc.py	0.00%	9 Missing ⚠️
...n/src/datahub/ingestion/source/gc/query_cleanup.py	98.80%	1 Missing ⚠️

📢 Thoughts on this report? Let us know!

Add a Query Cleanup section to the datahub-gc connector docs describing the SYSTEM-query soft-delete first pass, its config, and the retention_days guidance relative to the soft-deleted-entities cleanup clock. Also add a query_cleanup block to the starter recipe. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

datahub-connector-tests · 2026-07-01T13:32:30Z

Connector Tests Results

All connector tests passed for commit b60f0ba

View full test logs →

To skip connector tests, add the skip-connector-tests label (org members only).

Autogenerated by the connector-tests CI pipeline.

- Prefix report flags with qc_ to avoid MRO field collision with SoftDeletedEntitiesReport in DataHubGcSourceReport - Cap deletions at exactly limit_entities_delete (>= instead of >) - Drop redundant num_system_queries_found counter - Track invalid-URN skips in the report (num_queries_invalid_urn) - Document the deletion limit as an approximate cap under concurrency - Convert tests from unittest.TestCase to pytest idioms Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Route soft deletes through the sink as StatusClass workunits (the same mechanism as stale-entity removal) instead of a bespoke ThreadPoolExecutor that called graph.delete_entity per URN. The sink batches and async-writes them (default ASYNC_BATCH), so query cleanup no longer maintains its own concurrency, backpressure, or emit-mode handling. Hard deletes have no bulk endpoint and are issued imperatively, one at a time. - Replace cleanup_queries() with a get_workunits() generator; datahub_gc now yields from it. - Drop max_workers, delay, futures_max_at_time, and emit_mode config, plus the futures/lock/_process_futures machinery they supported. Keep the runtime and deletion caps, enforced in the generator loop. - Report: drop num_queries_processed; add report_query_soft_deleted/ report_query_hard_deleted helpers; count at emit time; preview candidates in dry-run. Remove num_queries_invalid_urn in favor of report.warning, and count num_queries_found only for successfully parsed urns. - Update the datahub-gc docs and unit tests accordingly. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

The workunit refactor removed max_workers from QueryCleanupConfig; the bootstrap template still injected it, which would fail config validation. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Hard delete has no bulk or async path, so the per-URN blocking calls are fanned out across a ThreadPoolExecutor (max_workers) instead of run serially. Completed futures are drained on the calling thread, so report updates need no lock. Soft delete stays workunit-based. Restores the max_workers config (and its bootstrap template entry) for the hard path only. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

…atic failure Wrap the query hard-delete enumeration loop in try/finally so futures already submitted are still collected (and their completed deletes recorded in the report) when candidate enumeration raises mid-stream

Query cleanup now only soft-deletes stale SYSTEM queries and leaves the hard delete to the existing soft-deleted-entities cleanup pass, which also clears inbound references. Removes the hard_delete_entities/max_workers config, the worker-pool delete path, and the hard-deleted report counter, along with the corresponding docs and tests. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

…ch_size, limit_entities_delete, and runtime_limit_seconds

get_urns_by_filter issued scrollAcrossEntities with no sortInput, so candidates came back in arbitrary scroll order. When limit_entities_delete or runtime_limit_seconds tripped mid-stream, a capped run deleted an arbitrary subset rather than the oldest, and dry-run previews showed an arbitrary slice. Thread an optional sort_by/sort_order through get_urns_by_filter and have query cleanup request lastModifiedAt ascending so the most-stale queries drain first. The backend appends urn ascending as a tiebreaker, so search_after scroll pagination stays stable. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

… configs Remove hard_delete_entities/max_workers from the query_cleanup block of the datahub-gc bootstrap recipe; QueryCleanupConfig (extra="forbid") does not define them, so parsing the recipe would raise a ValidationError and crash the source. These were leftovers from the reverted hard-delete work. Also: document that limit_entities_delete=null disables the cap, use an explicit `is not None` guard, drop the always-truthy runtime_limit_seconds guard, update the dry_run and sort_order docstrings, and remove an unused import in the query cleanup test. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Bump the default age cutoff for SYSTEM query soft-deletion from 30 to 90 days in QueryCleanupConfig and the datahub-gc bootstrap recipe, and update the connector doc examples to match. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

sgomezvillamor · 2026-07-02T14:40:49Z

@@ -770,6 +771,7 @@ def test_get_urns_with_skip_cache() -> None:
                "batchSize": unittest.mock.ANY,
                "scrollId": None,
                "skipCache": True,
+                "sortInput": None,


may we also add a test case with non-null sortInput ?

sgomezvillamor · 2026-07-02T14:45:42Z

Reviewed the implementation — the approach is clean and well-scoped. Server-side selection (source == SYSTEM AND lastModifiedAt < cutoff) with no per-entity lookups scales well, the lazy generator + early break honors the limits without over-fetching, and delegating hard-delete to the existing SoftDeletedEntitiesCleanup pass keeps this pass minimal. I also confirmed the sorted deep-scroll is safe: ESUtils.buildSortOrder appends urn ascending as a tiebreaker when the sort criteria don't already include it, so oldest-first paging over scrollAcrossEntities is stable. The qc_-prefixed report flags correctly avoid colliding with SoftDeletedEntitiesReport's runtime_limit_reached/deletion_limit_reached in the merged DataHubGcSourceReport.

Two things worth addressing:

1. The PR description doesn't match the shipped code. The description documents hard_delete_entities, max_workers, a "Hard delete via a worker pool" path, a get_workunits() that "dispatches: dry-run preview, parallel hard delete, or soft-delete", "10 passing … parallel hard delete", and retention_days (30). None of that is in the diff — QueryCleanupConfig has only enabled, retention_days (default 90), batch_size, limit_entities_delete, runtime_limit_seconds; there's no hard-delete path; and the test file has 9 tests, none for hard delete. The soft-delete-only design is actually the cleaner choice, so this is a matter of updating the description to match rather than adding the missing code.

2. Dry-run ignores limit_entities_delete. In _preview_candidates, the stop condition calls _deletion_limit_reached(), which tests self.report.num_queries_soft_deleted — but that counter is never incremented during a dry run, so it stays 0 and the cap never triggers. A dry run therefore scrolls all matching candidates (bounded only by runtime_limit_seconds), and num_queries_found will overstate what a real capped run would delete — contradicting the intent that the dry-run preview reflect a real run. Suggest counting previewed candidates against limit_entities_delete in the dry-run path too.

Minor/optional: num_queries_found counts one past the cap in the real path (incremented before the _deletion_limit_reached() check, so found == limit + 1 while soft_deleted == limit); and the "limit_entities_delete=None disables the cap" behavior is only in a code comment that points to the field description, which doesn't actually mention it.

Generated by Claude Code

sgomezvillamor

LGTM

As raised internally, just pending on final decision on default value in the bootstra mcp

github-actions Bot added ingestion PR or Issue related to the ingestion of metadata devops PR or Issue related to DataHub backend & deployment labels Jul 1, 2026

github-actions Bot deployed to datahub-wheels (Preview) July 1, 2026 12:44 View deployment

github-actions Bot deployed to datahub-wheels (Preview) July 1, 2026 12:50 View deployment

vercel Bot deployed to Preview July 1, 2026 13:03 View deployment

github-actions Bot deployed to datahub-wheels (Preview) July 1, 2026 13:42 View deployment

vercel Bot deployed to Preview July 1, 2026 13:54 View deployment

feat(ingest/gc): add query processing count and update cleanup tests

b50c5ce

github-actions Bot deployed to datahub-wheels (Preview) July 1, 2026 17:58 View deployment

vercel Bot deployed to Preview July 1, 2026 18:11 View deployment

aviraj-gour marked this pull request as ready for review July 2, 2026 04:46

aviraj-gour and others added 2 commits July 2, 2026 11:23

fix(ingest/gc): drop removed max_workers from query_cleanup bootstrap

87503bd

The workunit refactor removed max_workers from QueryCleanupConfig; the bootstrap template still injected it, which would fail config validation. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

github-actions Bot deployed to datahub-wheels (Preview) July 2, 2026 05:57 View deployment

aviraj-gour marked this pull request as draft July 2, 2026 06:05

github-actions Bot deployed to datahub-wheels (Preview) July 2, 2026 06:08 View deployment

vercel Bot deployed to Preview July 2, 2026 06:21 View deployment

treff7es reviewed Jul 2, 2026

View reviewed changes

Comment thread metadata-ingestion/src/datahub/ingestion/source/gc/query_cleanup.py

aviraj-gour and others added 2 commits July 2, 2026 12:34

sgomezvillamor reviewed Jul 2, 2026

View reviewed changes

Comment thread metadata-ingestion/src/datahub/ingestion/source/gc/query_cleanup.py Outdated

fix(ingest/gc): add minimum value constraints for retention_days, bat…

f29c9eb

…ch_size, limit_entities_delete, and runtime_limit_seconds

github-actions Bot deployed to datahub-wheels (Preview) July 2, 2026 07:38 View deployment

vercel Bot deployed to Preview July 2, 2026 07:50 View deployment

aviraj-gour force-pushed the feat/datahub-gc-query branch from 78c2d56 to 7b160fc Compare July 2, 2026 07:57

github-actions Bot deployed to datahub-wheels (Preview) July 2, 2026 07:59 View deployment

vercel Bot deployed to Preview July 2, 2026 08:12 View deployment

github-actions Bot deployed to datahub-wheels (Preview) July 2, 2026 08:18 View deployment

github-actions Bot deployed to datahub-wheels (Preview) July 2, 2026 08:25 View deployment

aviraj-gour marked this pull request as ready for review July 2, 2026 08:33

vercel Bot deployed to Preview July 2, 2026 08:38 View deployment

fix(tests): add sortInput field to get_urns test cases

b60f0ba

github-actions Bot deployed to datahub-wheels (Preview) July 2, 2026 08:45 View deployment

vercel Bot deployed to Preview July 2, 2026 08:58 View deployment

maggiehays added the needs-review Label for PRs that need review from a maintainer. label Jul 2, 2026

sgomezvillamor reviewed Jul 2, 2026

View reviewed changes

maggiehays added pending-submitter-response Issue/request has been reviewed but requires a response from the submitter and removed needs-review Label for PRs that need review from a maintainer. labels Jul 2, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

feat(ingest/gc): add query cleanup pass for stale SYSTEM queries#18120

feat(ingest/gc): add query cleanup pass for stale SYSTEM queries#18120
aviraj-gour wants to merge 14 commits into
masterfrom
feat/datahub-gc-query

aviraj-gour commented Jul 1, 2026 •

edited

Loading

Uh oh!

codecov Bot commented Jul 1, 2026 •

edited

Loading

Uh oh!

datahub-connector-tests Bot commented Jul 1, 2026 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

sgomezvillamor Jul 2, 2026

Uh oh!

sgomezvillamor commented Jul 2, 2026

Uh oh!

sgomezvillamor left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Uh oh!

Conversation

aviraj-gour commented Jul 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Approach

Changes

Config

Testing

Checklist

Uh oh!

codecov Bot commented Jul 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

datahub-connector-tests Bot commented Jul 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Connector Tests Results

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

sgomezvillamor Jul 2, 2026

Choose a reason for hiding this comment

Uh oh!

sgomezvillamor commented Jul 2, 2026

Uh oh!

sgomezvillamor left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

aviraj-gour commented Jul 1, 2026 •

edited

Loading

codecov Bot commented Jul 1, 2026 •

edited

Loading

datahub-connector-tests Bot commented Jul 1, 2026 •

edited

Loading