Skip to content

Commit

Permalink
ref(backup): Refine dangling model resolution
Browse files Browse the repository at this point in the history
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
  • Loading branch information
azaslavsky committed Oct 7, 2023
1 parent 63df56c commit 11fcf1d
Show file tree
Hide file tree
Showing 2 changed files with 59 additions and 40 deletions.
80 changes: 40 additions & 40 deletions fixtures/backup/model_dependencies/detailed.json
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@
]
},
"hybridcloud.regioncacheversion": {
"dangling": true,
"dangling": false,
"foreign_keys": {},
"model": "hybridcloud.regioncacheversion",
"relocation_scope": "Excluded",
Expand All @@ -106,7 +106,7 @@
]
},
"nodestore.node": {
"dangling": true,
"dangling": false,
"foreign_keys": {},
"model": "nodestore.node",
"relocation_scope": "Excluded",
Expand Down Expand Up @@ -177,7 +177,7 @@
"uniques": []
},
"sentry.actor": {
"dangling": true,
"dangling": false,
"foreign_keys": {
"team": {
"kind": "FlexibleForeignKey",
Expand Down Expand Up @@ -206,7 +206,7 @@
]
},
"sentry.alertrule": {
"dangling": true,
"dangling": false,
"foreign_keys": {
"organization": {
"kind": "FlexibleForeignKey",
Expand Down Expand Up @@ -237,7 +237,7 @@
]
},
"sentry.alertruleactivity": {
"dangling": true,
"dangling": false,
"foreign_keys": {
"alert_rule": {
"kind": "FlexibleForeignKey",
Expand Down Expand Up @@ -291,7 +291,7 @@
]
},
"sentry.alertruletrigger": {
"dangling": true,
"dangling": false,
"foreign_keys": {
"alert_rule": {
"kind": "FlexibleForeignKey",
Expand All @@ -313,7 +313,7 @@
]
},
"sentry.alertruletriggeraction": {
"dangling": true,
"dangling": false,
"foreign_keys": {
"alert_rule_trigger": {
"kind": "FlexibleForeignKey",
Expand Down Expand Up @@ -795,7 +795,7 @@
]
},
"sentry.broadcast": {
"dangling": true,
"dangling": false,
"foreign_keys": {},
"model": "sentry.broadcast",
"relocation_scope": "Excluded",
Expand Down Expand Up @@ -918,7 +918,7 @@
]
},
"sentry.controlfile": {
"dangling": true,
"dangling": false,
"foreign_keys": {},
"model": "sentry.controlfile",
"relocation_scope": "Excluded",
Expand All @@ -929,7 +929,7 @@
"uniques": []
},
"sentry.controlfileblob": {
"dangling": true,
"dangling": false,
"foreign_keys": {},
"model": "sentry.controlfileblob",
"relocation_scope": "Excluded",
Expand All @@ -944,7 +944,7 @@
]
},
"sentry.controlfileblobindex": {
"dangling": true,
"dangling": false,
"foreign_keys": {
"blob": {
"kind": "FlexibleForeignKey",
Expand Down Expand Up @@ -1014,7 +1014,7 @@
]
},
"sentry.controloutbox": {
"dangling": true,
"dangling": false,
"foreign_keys": {},
"model": "sentry.controloutbox",
"relocation_scope": "Excluded",
Expand All @@ -1025,7 +1025,7 @@
"uniques": []
},
"sentry.controltombstone": {
"dangling": true,
"dangling": false,
"foreign_keys": {},
"model": "sentry.controltombstone",
"relocation_scope": "Excluded",
Expand Down Expand Up @@ -1243,7 +1243,7 @@
"uniques": []
},
"sentry.deletedorganization": {
"dangling": true,
"dangling": false,
"foreign_keys": {},
"model": "sentry.deletedorganization",
"relocation_scope": "Excluded",
Expand All @@ -1254,7 +1254,7 @@
"uniques": []
},
"sentry.deletedproject": {
"dangling": true,
"dangling": false,
"foreign_keys": {
"organization_id": {
"kind": "ImplicitForeignKey",
Expand All @@ -1271,7 +1271,7 @@
"uniques": []
},
"sentry.deletedteam": {
"dangling": true,
"dangling": false,
"foreign_keys": {
"organization_id": {
"kind": "ImplicitForeignKey",
Expand Down Expand Up @@ -1391,7 +1391,7 @@
]
},
"sentry.docintegration": {
"dangling": true,
"dangling": false,
"foreign_keys": {},
"model": "sentry.docintegration",
"relocation_scope": "Excluded",
Expand All @@ -1406,7 +1406,7 @@
]
},
"sentry.docintegrationavatar": {
"dangling": true,
"dangling": false,
"foreign_keys": {
"control_file_id": {
"kind": "ImplicitForeignKey",
Expand Down Expand Up @@ -1738,7 +1738,7 @@
]
},
"sentry.file": {
"dangling": true,
"dangling": false,
"foreign_keys": {
"blob": {
"kind": "FlexibleForeignKey",
Expand All @@ -1755,7 +1755,7 @@
"uniques": []
},
"sentry.fileblob": {
"dangling": true,
"dangling": false,
"foreign_keys": {},
"model": "sentry.fileblob",
"relocation_scope": "Excluded",
Expand All @@ -1770,7 +1770,7 @@
]
},
"sentry.fileblobindex": {
"dangling": true,
"dangling": false,
"foreign_keys": {
"blob": {
"kind": "FlexibleForeignKey",
Expand Down Expand Up @@ -2046,7 +2046,7 @@
]
},
"sentry.grouphash": {
"dangling": true,
"dangling": false,
"foreign_keys": {
"group": {
"kind": "FlexibleForeignKey",
Expand Down Expand Up @@ -2535,7 +2535,7 @@
]
},
"sentry.identityprovider": {
"dangling": true,
"dangling": false,
"foreign_keys": {},
"model": "sentry.identityprovider",
"relocation_scope": "Excluded",
Expand Down Expand Up @@ -2772,7 +2772,7 @@
]
},
"sentry.integrationfeature": {
"dangling": true,
"dangling": false,
"foreign_keys": {},
"model": "sentry.integrationfeature",
"relocation_scope": "Excluded",
Expand Down Expand Up @@ -2874,7 +2874,7 @@
]
},
"sentry.metricskeyindexer": {
"dangling": true,
"dangling": false,
"foreign_keys": {},
"model": "sentry.metricskeyindexer",
"relocation_scope": "Excluded",
Expand Down Expand Up @@ -3010,7 +3010,7 @@
"uniques": []
},
"sentry.monitorlocation": {
"dangling": true,
"dangling": false,
"foreign_keys": {},
"model": "sentry.monitorlocation",
"relocation_scope": "Excluded",
Expand Down Expand Up @@ -3102,7 +3102,7 @@
"uniques": []
},
"sentry.notificationsetting": {
"dangling": true,
"dangling": false,
"foreign_keys": {
"target_id": {
"kind": "HybridCloudForeignKey",
Expand Down Expand Up @@ -3137,7 +3137,7 @@
]
},
"sentry.notificationsettingoption": {
"dangling": true,
"dangling": false,
"foreign_keys": {
"team_id": {
"kind": "HybridCloudForeignKey",
Expand Down Expand Up @@ -3167,7 +3167,7 @@
]
},
"sentry.notificationsettingprovider": {
"dangling": true,
"dangling": false,
"foreign_keys": {
"team_id": {
"kind": "HybridCloudForeignKey",
Expand Down Expand Up @@ -3854,7 +3854,7 @@
]
},
"sentry.projectdebugfile": {
"dangling": true,
"dangling": false,
"foreign_keys": {
"file": {
"kind": "FlexibleForeignKey",
Expand Down Expand Up @@ -4301,7 +4301,7 @@
]
},
"sentry.regionoutbox": {
"dangling": true,
"dangling": false,
"foreign_keys": {},
"model": "sentry.regionoutbox",
"relocation_scope": "Excluded",
Expand All @@ -4312,7 +4312,7 @@
"uniques": []
},
"sentry.regionscheduleddeletion": {
"dangling": true,
"dangling": false,
"foreign_keys": {},
"model": "sentry.regionscheduleddeletion",
"relocation_scope": "Excluded",
Expand All @@ -4332,7 +4332,7 @@
]
},
"sentry.regiontombstone": {
"dangling": true,
"dangling": false,
"foreign_keys": {},
"model": "sentry.regiontombstone",
"relocation_scope": "Excluded",
Expand Down Expand Up @@ -4848,7 +4848,7 @@
"uniques": []
},
"sentry.rulesnooze": {
"dangling": true,
"dangling": false,
"foreign_keys": {
"alert_rule": {
"kind": "FlexibleForeignKey",
Expand Down Expand Up @@ -4889,7 +4889,7 @@
]
},
"sentry.savedsearch": {
"dangling": true,
"dangling": false,
"foreign_keys": {
"organization": {
"kind": "FlexibleForeignKey",
Expand All @@ -4911,7 +4911,7 @@
"uniques": []
},
"sentry.scheduleddeletion": {
"dangling": true,
"dangling": false,
"foreign_keys": {},
"model": "sentry.scheduleddeletion",
"relocation_scope": "Excluded",
Expand Down Expand Up @@ -5152,7 +5152,7 @@
]
},
"sentry.servicehook": {
"dangling": true,
"dangling": false,
"foreign_keys": {
"application_id": {
"kind": "HybridCloudForeignKey",
Expand Down Expand Up @@ -5232,7 +5232,7 @@
"uniques": []
},
"sentry.snubaqueryeventtype": {
"dangling": true,
"dangling": false,
"foreign_keys": {
"snuba_query": {
"kind": "FlexibleForeignKey",
Expand Down Expand Up @@ -5626,7 +5626,7 @@
"uniques": []
},
"sessions.session": {
"dangling": true,
"dangling": false,
"foreign_keys": {},
"model": "sessions.session",
"relocation_scope": "Excluded",
Expand All @@ -5641,7 +5641,7 @@
]
},
"sites.site": {
"dangling": true,
"dangling": false,
"foreign_keys": {},
"model": "sites.site",
"relocation_scope": "Excluded",
Expand Down
19 changes: 19 additions & 0 deletions src/sentry/backup/dependencies.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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

Expand Down

0 comments on commit 11fcf1d

Please sign in to comment.