Skip to content

FIXES #20341 : Add reverse ingestion Workflow cleanup to Data Retention app#26300

Open
Siddhanttimeline wants to merge 16 commits intoopen-metadata:mainfrom
Siddhanttimeline:feature/workflow-retention-20341
Open

FIXES #20341 : Add reverse ingestion Workflow cleanup to Data Retention app#26300
Siddhanttimeline wants to merge 16 commits intoopen-metadata:mainfrom
Siddhanttimeline:feature/workflow-retention-20341

Conversation

@Siddhanttimeline
Copy link
Copy Markdown
Contributor

@Siddhanttimeline Siddhanttimeline commented Mar 6, 2026

Describe your changes:

Fixes #20341

Data Retention – Workflow Cleanup for Reverse Metadata

Summary

This change extends the Data Retention application to automatically clean up Workflow entities created by reverse metadata (reverse ingestion) workflows, preventing unbounded growth of these records.

Changes

Retention configuration

  • Schema updates

    • Extended the internal retention configuration schema to add reverseIngestionWorkflowRetentionPeriod (days).
    • Marked reverseIngestionWorkflowRetentionPeriod as required, with a default of 30 days.
  • Default app configuration

    • Updated the default Data Retention app configuration to include:
      • auditLogRetentionPeriod: 90
      • reverseIngestionWorkflowRetentionPeriod: 30.

Backend cleanup logic

  • DataRetention app

    • Injected CollectionDAO.WorkflowDAO into DataRetention.
    • Extended stats initialization to include:
      • reverse_ingestion_workflows.
    • Read the new configuration value via:
      • config.getReverseIngestionWorkflowRetentionPeriod().
    • Added a new cleanup step:
      • cleanReverseIngestionWorkflows(int retentionPeriod):
        • Computes a cutoff timestamp from the retention period.
        • Uses executeWithStatsTracking("reverse_ingestion_workflows", ...) to batch‑delete from automations_workflow where:
          • workflowType = 'REVERSE_INGESTION'
          • status IN ('Successful', 'Failed')
          • updatedAt is older than the cutoff.
        • Updates job and entity stats with the number of deleted workflows.
  • DAO changes

    • Added to CollectionDAO.WorkflowDAO:
      • int deleteReverseIngestionWorkflowsBeforeCutoff(long cutoffTs, int limit);
    • Implemented database‑specific batched delete queries:
      • MySQL
        • Deletes directly from automations_workflow filtered by:
          • workflowType = 'REVERSE_INGESTION'
          • status IN ('Successful', 'Failed')
          • updatedAt < :cutoffTs
        • Uses ORDER BY updatedAt LIMIT :limit for batching.
      • Postgres
        • Deletes using ctid subselect from automations_workflow with the same filters on:
          • workflowtype, status, updatedat
        • Uses ORDER BY updatedat LIMIT :limit for safe batched deletion.

I worked on ... because ...

Type of change:

  • Bug fix
  • Improvement
  • New feature
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation

Checklist:

  • I have read the CONTRIBUTING document.
  • My PR title is Fixes <issue-number>: <short explanation>
  • I have commented on my code, particularly in hard-to-understand areas.
  • For JSON Schema changes: I updated the migration scripts or explained why it is not needed.

On each run, deletes, reverse-ingestion workflows (workflowType = 'REVERSE_INGESTION') whose status is Successful or Failed and whose updatedAt is older than the configured retention period, in batches of 10,000.
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Mar 6, 2026

Hi there 👋 Thanks for your contribution!

The OpenMetadata team will review the PR shortly! Once it has been labeled as safe to test, the CI workflows
will start executing and we'll be able to make sure everything is working as expected.

Let us know if you need any help!

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Mar 6, 2026

Hi there 👋 Thanks for your contribution!

The OpenMetadata team will review the PR shortly! Once it has been labeled as safe to test, the CI workflows
will start executing and we'll be able to make sure everything is working as expected.

Let us know if you need any help!

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Mar 7, 2026

Hi there 👋 Thanks for your contribution!

The OpenMetadata team will review the PR shortly! Once it has been labeled as safe to test, the CI workflows
will start executing and we'll be able to make sure everything is working as expected.

Let us know if you need any help!

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Mar 14, 2026

🔴 Playwright Results — 1 failure(s), 20 flaky

✅ 3642 passed · ❌ 1 failed · 🟡 20 flaky · ⏭️ 111 skipped

Shard Passed Failed Flaky Skipped
🔴 Shard 1 454 1 3 26
🟡 Shard 2 648 0 3 7
🟡 Shard 3 654 0 6 1
🟡 Shard 4 630 0 4 27
🟡 Shard 5 609 0 2 42
🟡 Shard 6 647 0 2 8

Genuine Failures (failed on all attempts)

Features/DataAssetRulesDisabled.spec.ts › Verify the Database entity item action after rules disabled (shard 1)
Error: �[2mexpect(�[22m�[31mlocator�[39m�[2m).�[22mtoContainText�[2m(�[22m�[32mexpected�[39m�[2m)�[22m failed

Locator: getByTestId('domain-link')
Expected substring: �[32m"PW Domain �[7m442951fe�[27m"�[39m
Received string:    �[31m"PW Domain �[7m3c723977�[27m"�[39m
Timeout: 15000ms

Call log:
�[2m  - Expect "toContainText" with timeout 15000ms�[22m
�[2m  - waiting for getByTestId('domain-link')�[22m
�[2m    19 × locator resolved to <a data-testid="domain-link" href="/domain/%22PW%25domain.3c723977%22" class="no-underline domain-link domain-link-text font-medium text-sm render-domain-lebel-style">PW Domain 3c723977</a>�[22m
�[2m       - unexpected value "PW Domain 3c723977"�[22m

🟡 20 flaky test(s) (passed on retry)
  • Features/DataAssetRulesDisabled.spec.ts › Verify the Database Schema entity item action after rules disabled (shard 1, 1 retry)
  • Features/CustomizeDetailPage.spec.ts › Domain - customization should work (shard 1, 1 retry)
  • Pages/UserCreationWithPersona.spec.ts › Create user with persona and verify on profile (shard 1, 1 retry)
  • Features/BulkEditEntity.spec.ts › Glossary (shard 2, 1 retry)
  • Features/ChangeSummaryBadge.spec.ts › AI badge should NOT appear for manually-edited descriptions (shard 2, 1 retry)
  • Features/Glossary/GlossaryWorkflow.spec.ts › should inherit reviewers from glossary when term is created (shard 2, 1 retry)
  • Features/RestoreEntityInheritedFields.spec.ts › Validate restore with Inherited domain and data products assigned (shard 3, 1 retry)
  • Features/RestoreEntityInheritedFields.spec.ts › Validate restore with Inherited domain and data products assigned (shard 3, 1 retry)
  • Features/RestoreEntityInheritedFields.spec.ts › Validate restore with Inherited domain and data products assigned (shard 3, 2 retries)
  • Features/RestoreEntityInheritedFields.spec.ts › Validate restore with Inherited domain and data products assigned (shard 3, 1 retry)
  • Features/RTL.spec.ts › Verify Following widget functionality (shard 3, 1 retry)
  • Features/UserProfileOnlineStatus.spec.ts › Should not show online status for inactive users (shard 3, 1 retry)
  • Pages/Customproperties-part2.spec.ts › entityReferenceList shows item count, scrollable list, no expand toggle (shard 4, 1 retry)
  • Pages/DataContracts.spec.ts › Create Data Contract and validate for ApiEndpoint (shard 4, 1 retry)
  • Pages/DataContracts.spec.ts › Create Data Contract and validate for Worksheet (shard 4, 1 retry)
  • Pages/Entity.spec.ts › User as Owner with unsorted list (shard 4, 1 retry)
  • Pages/EntityDataConsumer.spec.ts › Tier Add, Update and Remove (shard 5, 1 retry)
  • Pages/Glossary.spec.ts › Add and Remove Assets (shard 5, 2 retries)
  • Pages/Lineage/LineageFilters.spec.ts › Verify lineage schema filter selection (shard 6, 1 retry)
  • Pages/Users.spec.ts › Permissions for table details page for Data Consumer (shard 6, 1 retry)

📦 Download artifacts

How to debug locally
# Download playwright-test-results-<shard> artifact and unzip
npx playwright show-trace path/to/trace.zip    # view trace

@github-actions
Copy link
Copy Markdown
Contributor

⚠️ TypeScript Types Need Update

The generated TypeScript types are out of sync with the JSON schema changes.

Since this is a pull request from a forked repository, the types cannot be automatically committed.
Please generate and commit the types manually:

cd openmetadata-ui/src/main/resources/ui
./json2ts-generate-all.sh -l true
git add src/generated/
git commit -m "Update generated TypeScript types"
git push

After pushing the changes, this check will pass automatically.

@github-actions
Copy link
Copy Markdown
Contributor

⚠️ TypeScript Types Need Update

The generated TypeScript types are out of sync with the JSON schema changes.

Since this is a pull request from a forked repository, the types cannot be automatically committed.
Please generate and commit the types manually:

cd openmetadata-ui/src/main/resources/ui
./json2ts-generate-all.sh -l true
git add src/generated/
git commit -m "Update generated TypeScript types"
git push

After pushing the changes, this check will pass automatically.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 4, 2026

⚠️ TypeScript Types Need Update

The generated TypeScript types are out of sync with the JSON schema changes.

Since this is a pull request from a forked repository, the types cannot be automatically committed.
Please generate and commit the types manually:

cd openmetadata-ui/src/main/resources/ui
./json2ts-generate-all.sh -l true
git add src/generated/
git commit -m "Update generated TypeScript types"
git push

After pushing the changes, this check will pass automatically.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 6, 2026

⚠️ TypeScript Types Need Update

The generated TypeScript types are out of sync with the JSON schema changes.

Since this is a pull request from a forked repository, the types cannot be automatically committed.
Please generate and commit the types manually:

cd openmetadata-ui/src/main/resources/ui
./json2ts-generate-all.sh -l true
git add src/generated/
git commit -m "Update generated TypeScript types"
git push

After pushing the changes, this check will pass automatically.

@gitar-bot
Copy link
Copy Markdown

gitar-bot Bot commented Apr 17, 2026

Code Review ✅ Approved 3 resolved / 3 findings

Integrates reverse ingestion workflow cleanup into the Data Retention app, resolving potential replication failures, missing configuration fields, and redundant test files.

✅ 3 resolved
Edge Case: MySQL DELETE with ORDER BY may fail under row-based replication

📄 openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/CollectionDAO.java:8413
The MySQL variant of deleteReverseIngestionWorkflowsBeforeCutoff uses DELETE ... ORDER BY updatedAt LIMIT :limit, which is flagged as unsafe for statement-based replication (MySQL warning 1786). While this follows the existing pattern used by deleteOldRecords in the same codebase, it's worth noting that if the deployment uses statement-based or mixed-mode replication, this could produce warnings or non-deterministic results. The PostgreSQL variant correctly uses a ctid IN (SELECT ...) subquery.

Quality: Empty test file added with no actual tests

📄 openmetadata-service/src/test/java/org/openmetadata/service/apps/bundles/dataRetention/DataRetentionAppTest.java:1
The new file DataRetentionAppTest.java is completely empty (just a blank line). This means the new cleanReverseIngestionWorkflows logic and the deleteReverseIngestionWorkflowsBeforeCutoff SQL query have zero test coverage. At minimum, there should be tests verifying:

  1. The SQL correctly filters by workflowType = 'REVERSE_INGESTION' and terminal statuses.
  2. The batch deletion loop terminates correctly.
  3. The retention period cutoff calculation is correct.

Other cleanup methods (audit logs, change events, etc.) appear to lack dedicated unit tests too, but since this is a new file explicitly created for tests, it should contain actual test cases.

Bug: NPE on existing configs missing new retention period fields

📄 openmetadata-service/src/main/java/org/openmetadata/service/apps/bundles/dataRetention/DataRetention.java:168 📄 openmetadata-service/src/main/java/org/openmetadata/service/apps/bundles/dataRetention/DataRetention.java:173 📄 openmetadata-spec/src/main/resources/json/schema/entity/applications/configuration/internal/dataRetentionConfiguration.json:21 📄 openmetadata-spec/src/main/resources/json/schema/entity/applications/configuration/internal/dataRetentionConfiguration.json:27
Lines 168 and 173 auto-unbox the return value of getAuditLogRetentionPeriod() and getReverseIngestionWorkflowRetentionPeriod() to primitive int. For existing DataRetention app instances whose configuration was persisted before this change, the stored JSON won't contain these new fields. When Jackson deserializes the stored config, these fields will be null (Jackson overwrites the jsonschema2pojo default initializer). The auto-unboxing of null to int will throw a NullPointerException, crashing the entire Data Retention app run — meaning no cleanup happens at all (not just for the new workflows, but also for change events, threads, test results, etc.).

Options

Display: compact → Showing less information.

Comment with these commands to change:

Compact
gitar display:verbose         

Was this helpful? React with 👍 / 👎 | Gitar

@github-actions
Copy link
Copy Markdown
Contributor

⚠️ TypeScript Types Need Update

The generated TypeScript types are out of sync with the JSON schema changes.

Since this is a pull request from a forked repository, the types cannot be automatically committed.
Please generate and commit the types manually:

cd openmetadata-ui/src/main/resources/ui
./json2ts-generate-all.sh -l true
git add src/generated/
git commit -m "Update generated TypeScript types"
git push

After pushing the changes, this check will pass automatically.

@sonarqubecloud
Copy link
Copy Markdown

@sonarqubecloud
Copy link
Copy Markdown

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

safe to test Add this label to run secure Github workflows on PRs

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Retention App] Add workflows retention

2 participants