Skip to content

fix(aci milestone 3): clean up orphaned objects from workflow engine #90796

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
May 2, 2025

Conversation

mifu67
Copy link
Contributor

@mifu67 mifu67 commented May 1, 2025

  • Find orphaned data condition groups that are not connected to any detector or workflow.
  • Delete orphaned actions that are connected to the orphaned data condition groups. This deletes the AARTA and DCGA lookup entries as well.
  • Delete the orphaned data condition groups, which also deletes their associated data conditions.

After running this cleanup, we will fix an issue where some trigger actions had multiple entries in the AARTA table, which preventing users from editing alerts with this error.

@mifu67 mifu67 requested review from saponifi3d and ceorourke May 1, 2025 22:15
@mifu67 mifu67 requested review from a team as code owners May 1, 2025 22:15
@github-actions github-actions bot added the Scope: Backend Automatically applied to PRs that change backend components label May 1, 2025
# is a schema change, it's completely safe to run the operation after the code has deployed.
# Once deployed, run these manually via: https://develop.sentry.dev/database-migrations/#migration-deployment

is_post_deployment = True
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Marked post-deploy, but it probably doesn't have to be.

Copy link
Contributor

github-actions bot commented May 1, 2025

This PR has a migration; here is the generated SQL for src/sentry/workflow_engine/migrations/0054_clean_up_orphaned_metric_alert_objects.py

for 0054_clean_up_orphaned_metric_alert_objects in workflow_engine

--
-- Raw Python operation
--
-- THIS OPERATION CANNOT BE WRITTEN AS SQL

Copy link

codecov bot commented May 1, 2025

Codecov Report

Attention: Patch coverage is 64.28571% with 20 lines in your changes missing coverage. Please review.

✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
...est_0054_clean_up_orphaned_metric_alert_objects.py 37.93% 18 Missing ⚠️
...ons/0054_clean_up_orphaned_metric_alert_objects.py 92.59% 2 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff            @@
##           master   #90796    +/-   ##
========================================
  Coverage   87.80%   87.81%            
========================================
  Files       10283    10285     +2     
  Lines      583510   583651   +141     
  Branches    22569    22569            
========================================
+ Hits       512338   512510   +172     
+ Misses      70742    70711    -31     
  Partials      430      430            

Copy link
Member

@wedamija wedamija left a comment

Choose a reason for hiding this comment

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

How many rows are going to be affected here?

Comment on lines 44 to 46
with transaction.atomic(router.db_for_write(Action)):
orphaned_actions.delete()
orphaned_dcgs.delete()
Copy link
Member

Choose a reason for hiding this comment

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

Typically we prefer to not run large batch deletions like this, since it could potentially run for a long time and will time out. Usually we prefer to take the slower route of just iterating through the rows individually and deleting.

@mifu67
Copy link
Contributor Author

mifu67 commented May 2, 2025

How many rows are going to be affected here?

@wedamija ~700 data condition groups and ~250 actions. I can change it to iterate and delete.

Comment on lines 44 to 48
with transaction.atomic(router.db_for_write(Action)):
for action in orphaned_actions:
action.delete()
for dcg in orphaned_dcgs:
dcg.delete()
Copy link
Member

Choose a reason for hiding this comment

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

I'd also recommend not wrapping this entire operation in a transaction, because all involved rows will be locked for the whole transaction. Maybe it doesn't matter too much since they're orphaned, but better to be safe.

If you want to make sure the rows related to each other are in a transaction that's fine, but you'll just need to associate them and have a smaller transaction for each set of related rows. I think it's probably safe enough to just skip it though.

)


class TestCleanUpOrphanedMetricAlertObjects(TestMigrations):
Copy link
Member

Choose a reason for hiding this comment

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

Let's comment this out now that it has proved this runs fine, just to make sure ci doesn't end up breaking.

@mifu67 mifu67 enabled auto-merge (squash) May 2, 2025 17:27
@mifu67 mifu67 merged commit 6c2b8b3 into master May 2, 2025
59 checks passed
@mifu67 mifu67 deleted the mifu67/aci/clean-up-orphaned-metric-alert-objects branch May 2, 2025 17:37
Copy link

sentry-io bot commented May 2, 2025

Suspect Issues

This pull request was deployed and Sentry observed the following issues:

  • ‼️ ProgrammingError: UndefinedTable('relation "workflow_engine_action" does not exist\nLINE 1: SELECT COUNT(*) AS "__count" FROM "workflow_engine_action" W...\n ^\n') sentry.workflow_engine.migrations.0054_clean_up... View Issue

Did you find this useful? React with a 👍 or 👎

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Scope: Backend Automatically applied to PRs that change backend components
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants