Skip to content

fix(ingest/gc): retain non-ingestion execution requests via source.type fallback#18258

Merged
rajatoss merged 7 commits into
masterfrom
fix/gc-execreq-source-type-retention
Jul 14, 2026
Merged

fix(ingest/gc): retain non-ingestion execution requests via source.type fallback#18258
rajatoss merged 7 commits into
masterfrom
fix/gc-execreq-source-type-retention

Conversation

@rajatoss

@rajatoss rajatoss commented Jul 8, 2026

Copy link
Copy Markdown
Member

No description provided.

…pe fallback

datahub-gc's execution_request_cleanup grouped execution requests for
retention solely by source.ingestionSource, and hard-deleted any request
without one as a "corrupted" record on every run - bypassing the
min/max-count, max-age, and RUNNING/PENDING safeguards.

Only ingestion runs populate ingestionSource. Non-ingestion execution
requests (assertion/monitor "observe" workloads, agent tasks, evals,
connection tests) legitimately set only source.type, so they were being
deleted immediately regardless of age or status.

Fall back to source.type when ingestionSource is absent so these records
get real, bucketed retention, and add per-source.type retention overrides
so non-ingestion requests can use a different (typically shorter) policy
than ingestion runs. A record is treated as corrupted only when it has
neither ingestionSource nor source.type.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@github-actions 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 8, 2026
@rajatoss rajatoss self-assigned this Jul 8, 2026
@codecov

codecov Bot commented Jul 8, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 97.67442% with 1 line in your changes missing coverage. Please review.
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
...b/ingestion/source/gc/execution_request_cleanup.py 97.67% 1 Missing ⚠️

📢 Thoughts on this report? Let us know!

Add a test asserting two distinct ingestion sources are retained in
independent buckets, guarding against a regression that collapses
grouping by source.type instead of ingestion source URN.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Comment thread metadata-ingestion/src/datahub/ingestion/source/gc/execution_request_cleanup.py Outdated
@maggiehays 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 9, 2026
- Drop the dead corrupted-record branch; unattributable records (no
  ingestion source and no source.type) now fall into a single UNKNOWN
  bucket capped at UNKNOWN_SOURCE_MAX_COUNT instead of being deleted.
- Make CleanupRecord.source_type non-defaulted, consistent with siblings.
- Keep the bootstrap recipe's source_type_overrides empty by default;
  deployments provide overrides via DATAHUB_GC_BOOTSTRAP_VALUES.
- Use a generic source.type in tests.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@datahub-connector-tests

datahub-connector-tests Bot commented Jul 11, 2026

Copy link
Copy Markdown

Connector Tests Results

All connector tests passed for commit 44cf417

View full test logs →

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

Autogenerated by the connector-tests CI pipeline.

Comment on lines 285 to +297
if key not in state:
state[key] = {}
state[key]["cutoffTimestamp"] = (
entry.requested_at - self.config.keep_history_max_milliseconds()
# Retention is resolved once per bucket, from the source.type of the
# record that opens it, so a per-type override can apply a distinct policy.
retention = self.config.get_retention(
entry.source_type or UNKNOWN_SOURCE_KEY
)
state[key] = _BucketState(
min_count=retention.min_count,
max_count=retention.max_count,
cutoff_timestamp=entry.requested_at - retention.max_milliseconds,
)

state[key]["count"] = state[key].get("count", 0) + 1
bucket = state[key]

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor suggestion, not a blocker. This mechanism might be simplified with defaultdict and a dedicated helper function to create a new bucket.

@rajatoss
rajatoss merged commit 9341088 into master Jul 14, 2026
74 checks passed
@rajatoss
rajatoss deleted the fix/gc-execreq-source-type-retention branch July 14, 2026 10:03
asikowitz pushed a commit that referenced this pull request Jul 14, 2026
…pe fallback (#18258)

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

devops PR or Issue related to DataHub backend & deployment ingestion PR or Issue related to the ingestion of metadata pending-submitter-merge

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants