From 11fcf1d06a6472851a74e8d78d3d9fe521c39ec8 Mon Sep 17 00:00:00 2001 From: Alex Zaslavsky Date: Sat, 30 Sep 2023 09:18:58 -0700 Subject: [PATCH] ref(backup): Refine dangling model resolution Certain models that do not have unambiguous relationship to any RelocationScope root model (ex: Organization, User, etc) are termed "dangling", because filtering them down is done by simply moving up the model dependency tree from the root to the leaves - we have to double back and look at these "dangling" innner leaves instead. See https://tinyurl.com/27z4x6tk for more info; the `SnubaQuery`, `TimeSeriesSnapshot`, and `Email` models are all dangling in this manner. This PR changes the dangling resolution logic a bit: all `Excluded` relocation scope models are no longer considered dangling. Additionally, a few models that didn't quite get over the hump for "dangling" resolution during status analysis, but are obviously so, are marked as such. Issue: getsentry/team-ospo#203 --- .../backup/model_dependencies/detailed.json | 80 +++++++++---------- src/sentry/backup/dependencies.py | 19 +++++ 2 files changed, 59 insertions(+), 40 deletions(-) diff --git a/fixtures/backup/model_dependencies/detailed.json b/fixtures/backup/model_dependencies/detailed.json index 7d10e1115ef476..dc78492264387a 100644 --- a/fixtures/backup/model_dependencies/detailed.json +++ b/fixtures/backup/model_dependencies/detailed.json @@ -91,7 +91,7 @@ ] }, "hybridcloud.regioncacheversion": { - "dangling": true, + "dangling": false, "foreign_keys": {}, "model": "hybridcloud.regioncacheversion", "relocation_scope": "Excluded", @@ -106,7 +106,7 @@ ] }, "nodestore.node": { - "dangling": true, + "dangling": false, "foreign_keys": {}, "model": "nodestore.node", "relocation_scope": "Excluded", @@ -177,7 +177,7 @@ "uniques": [] }, "sentry.actor": { - "dangling": true, + "dangling": false, "foreign_keys": { "team": { "kind": "FlexibleForeignKey", @@ -206,7 +206,7 @@ ] }, "sentry.alertrule": { - "dangling": true, + "dangling": false, "foreign_keys": { "organization": { "kind": "FlexibleForeignKey", @@ -237,7 +237,7 @@ ] }, "sentry.alertruleactivity": { - "dangling": true, + "dangling": false, "foreign_keys": { "alert_rule": { "kind": "FlexibleForeignKey", @@ -291,7 +291,7 @@ ] }, "sentry.alertruletrigger": { - "dangling": true, + "dangling": false, "foreign_keys": { "alert_rule": { "kind": "FlexibleForeignKey", @@ -313,7 +313,7 @@ ] }, "sentry.alertruletriggeraction": { - "dangling": true, + "dangling": false, "foreign_keys": { "alert_rule_trigger": { "kind": "FlexibleForeignKey", @@ -795,7 +795,7 @@ ] }, "sentry.broadcast": { - "dangling": true, + "dangling": false, "foreign_keys": {}, "model": "sentry.broadcast", "relocation_scope": "Excluded", @@ -918,7 +918,7 @@ ] }, "sentry.controlfile": { - "dangling": true, + "dangling": false, "foreign_keys": {}, "model": "sentry.controlfile", "relocation_scope": "Excluded", @@ -929,7 +929,7 @@ "uniques": [] }, "sentry.controlfileblob": { - "dangling": true, + "dangling": false, "foreign_keys": {}, "model": "sentry.controlfileblob", "relocation_scope": "Excluded", @@ -944,7 +944,7 @@ ] }, "sentry.controlfileblobindex": { - "dangling": true, + "dangling": false, "foreign_keys": { "blob": { "kind": "FlexibleForeignKey", @@ -1014,7 +1014,7 @@ ] }, "sentry.controloutbox": { - "dangling": true, + "dangling": false, "foreign_keys": {}, "model": "sentry.controloutbox", "relocation_scope": "Excluded", @@ -1025,7 +1025,7 @@ "uniques": [] }, "sentry.controltombstone": { - "dangling": true, + "dangling": false, "foreign_keys": {}, "model": "sentry.controltombstone", "relocation_scope": "Excluded", @@ -1243,7 +1243,7 @@ "uniques": [] }, "sentry.deletedorganization": { - "dangling": true, + "dangling": false, "foreign_keys": {}, "model": "sentry.deletedorganization", "relocation_scope": "Excluded", @@ -1254,7 +1254,7 @@ "uniques": [] }, "sentry.deletedproject": { - "dangling": true, + "dangling": false, "foreign_keys": { "organization_id": { "kind": "ImplicitForeignKey", @@ -1271,7 +1271,7 @@ "uniques": [] }, "sentry.deletedteam": { - "dangling": true, + "dangling": false, "foreign_keys": { "organization_id": { "kind": "ImplicitForeignKey", @@ -1391,7 +1391,7 @@ ] }, "sentry.docintegration": { - "dangling": true, + "dangling": false, "foreign_keys": {}, "model": "sentry.docintegration", "relocation_scope": "Excluded", @@ -1406,7 +1406,7 @@ ] }, "sentry.docintegrationavatar": { - "dangling": true, + "dangling": false, "foreign_keys": { "control_file_id": { "kind": "ImplicitForeignKey", @@ -1738,7 +1738,7 @@ ] }, "sentry.file": { - "dangling": true, + "dangling": false, "foreign_keys": { "blob": { "kind": "FlexibleForeignKey", @@ -1755,7 +1755,7 @@ "uniques": [] }, "sentry.fileblob": { - "dangling": true, + "dangling": false, "foreign_keys": {}, "model": "sentry.fileblob", "relocation_scope": "Excluded", @@ -1770,7 +1770,7 @@ ] }, "sentry.fileblobindex": { - "dangling": true, + "dangling": false, "foreign_keys": { "blob": { "kind": "FlexibleForeignKey", @@ -2046,7 +2046,7 @@ ] }, "sentry.grouphash": { - "dangling": true, + "dangling": false, "foreign_keys": { "group": { "kind": "FlexibleForeignKey", @@ -2535,7 +2535,7 @@ ] }, "sentry.identityprovider": { - "dangling": true, + "dangling": false, "foreign_keys": {}, "model": "sentry.identityprovider", "relocation_scope": "Excluded", @@ -2772,7 +2772,7 @@ ] }, "sentry.integrationfeature": { - "dangling": true, + "dangling": false, "foreign_keys": {}, "model": "sentry.integrationfeature", "relocation_scope": "Excluded", @@ -2874,7 +2874,7 @@ ] }, "sentry.metricskeyindexer": { - "dangling": true, + "dangling": false, "foreign_keys": {}, "model": "sentry.metricskeyindexer", "relocation_scope": "Excluded", @@ -3010,7 +3010,7 @@ "uniques": [] }, "sentry.monitorlocation": { - "dangling": true, + "dangling": false, "foreign_keys": {}, "model": "sentry.monitorlocation", "relocation_scope": "Excluded", @@ -3102,7 +3102,7 @@ "uniques": [] }, "sentry.notificationsetting": { - "dangling": true, + "dangling": false, "foreign_keys": { "target_id": { "kind": "HybridCloudForeignKey", @@ -3137,7 +3137,7 @@ ] }, "sentry.notificationsettingoption": { - "dangling": true, + "dangling": false, "foreign_keys": { "team_id": { "kind": "HybridCloudForeignKey", @@ -3167,7 +3167,7 @@ ] }, "sentry.notificationsettingprovider": { - "dangling": true, + "dangling": false, "foreign_keys": { "team_id": { "kind": "HybridCloudForeignKey", @@ -3854,7 +3854,7 @@ ] }, "sentry.projectdebugfile": { - "dangling": true, + "dangling": false, "foreign_keys": { "file": { "kind": "FlexibleForeignKey", @@ -4301,7 +4301,7 @@ ] }, "sentry.regionoutbox": { - "dangling": true, + "dangling": false, "foreign_keys": {}, "model": "sentry.regionoutbox", "relocation_scope": "Excluded", @@ -4312,7 +4312,7 @@ "uniques": [] }, "sentry.regionscheduleddeletion": { - "dangling": true, + "dangling": false, "foreign_keys": {}, "model": "sentry.regionscheduleddeletion", "relocation_scope": "Excluded", @@ -4332,7 +4332,7 @@ ] }, "sentry.regiontombstone": { - "dangling": true, + "dangling": false, "foreign_keys": {}, "model": "sentry.regiontombstone", "relocation_scope": "Excluded", @@ -4848,7 +4848,7 @@ "uniques": [] }, "sentry.rulesnooze": { - "dangling": true, + "dangling": false, "foreign_keys": { "alert_rule": { "kind": "FlexibleForeignKey", @@ -4889,7 +4889,7 @@ ] }, "sentry.savedsearch": { - "dangling": true, + "dangling": false, "foreign_keys": { "organization": { "kind": "FlexibleForeignKey", @@ -4911,7 +4911,7 @@ "uniques": [] }, "sentry.scheduleddeletion": { - "dangling": true, + "dangling": false, "foreign_keys": {}, "model": "sentry.scheduleddeletion", "relocation_scope": "Excluded", @@ -5152,7 +5152,7 @@ ] }, "sentry.servicehook": { - "dangling": true, + "dangling": false, "foreign_keys": { "application_id": { "kind": "HybridCloudForeignKey", @@ -5232,7 +5232,7 @@ "uniques": [] }, "sentry.snubaqueryeventtype": { - "dangling": true, + "dangling": false, "foreign_keys": { "snuba_query": { "kind": "FlexibleForeignKey", @@ -5626,7 +5626,7 @@ "uniques": [] }, "sessions.session": { - "dangling": true, + "dangling": false, "foreign_keys": {}, "model": "sessions.session", "relocation_scope": "Excluded", @@ -5641,7 +5641,7 @@ ] }, "sites.site": { - "dangling": true, + "dangling": false, "foreign_keys": {}, "model": "sites.site", "relocation_scope": "Excluded", diff --git a/src/sentry/backup/dependencies.py b/src/sentry/backup/dependencies.py index 8d671ad30bfa9d..b5318ceb5109fd 100644 --- a/src/sentry/backup/dependencies.py +++ b/src/sentry/backup/dependencies.py @@ -437,6 +437,22 @@ def dependencies() -> dict[NormalizedModelName, ModelRelations]: for model_name in relocation_root_models: model_dependencies_dict[model_name].dangling = False + # TODO(getsentry/team-ospo#190): In practice, we can treat `AlertRule`'s dependency on + # `Organization` as non-nullable, so mark it is non-dangling. This is a hack - we should figure + # out a more rigorous way to deduce this. The same applies to `Actor`, since each actor must + # reference at least one `User` or `Team`, neither of which are dangling. + model_dependencies_dict[NormalizedModelName("sentry.actor")].dangling = False + model_dependencies_dict[NormalizedModelName("sentry.alertrule")].dangling = False + + # TODO(getsentry/team-ospo#190): The same is basically true for the remaining models in this + # list: the schema defines all of their foreign keys as nullable, but since these models have no + # other models referencing them (ie, they are leaves on our dependency graph), we know that at + # least one of those nullable relations will be present on every model. + model_dependencies_dict[NormalizedModelName("sentry.savedsearch")].dangling = False + model_dependencies_dict[NormalizedModelName("sentry.servicehook")].dangling = False + model_dependencies_dict[NormalizedModelName("sentry.snubaqueryeventtype")].dangling = False + model_dependencies_dict[NormalizedModelName("sentry.rulesnooze")].dangling = False + # Now that all `ModelRelations` have been added to the `model_dependencies_dict`, we can circle # back and figure out which ones are actually dangling. We do this by marking all of the root # models non-dangling, then traversing from every other model to a (possible) root model @@ -449,6 +465,9 @@ def resolve_dangling(seen: Set[NormalizedModelName], model_name: NormalizedModel raise RuntimeError( f"Circular dependency: {model_name} cannot transitively reference itself" ) + if model_relations.relocation_scope == RelocationScope.Excluded: + model_relations.dangling = False + return model_relations.dangling if model_relations.dangling is not None: return model_relations.dangling