Skip to content

Commit 8930349

Browse files
wedamijawedamija
committed
Update docs on how to delete tables and columns
This uses the new operations from getsentry/sentry#81063 getsentry/sentry#81098
1 parent 78c9c00 commit 8930349

File tree

1 file changed

+139
-81
lines changed
  • develop-docs/api-server/application-domains/database-migrations

1 file changed

+139
-81
lines changed

‎develop-docs/api-server/application-domains/database-migrations/index.mdx

Lines changed: 139 additions & 81 deletions
Original file line numberDiff line numberDiff line change
@@ -145,144 +145,202 @@ This is complicated due to our deploy process. When we deploy, we run migrations
145145

146146
To avoid this, follow these steps:
147147

148-
- Mark the column as nullable if it isn't, and create a migration. (ex. `BoundedIntegerField(null=True)`)
149-
- Deploy.
150-
- Remove the column from the model, but in the migration make sure we only mark the state as removed.
151-
- Deploy.
152-
- Finally, create a migration that deletes the column.
148+
- (Optional, but ideal) Make a PR to remove all uses of the column in the codebase in a separate PR. This mostly helps with code cleanliness. This should be merged ahead of the migration prs, but we don't need to worry about whether it is deployed first.
149+
- Make another PR that:
150+
- Checks if the column is either not nullable, or doesn't have a db_default set. If either of these is true, then make it nullable via `null=True`.
151+
- If the column is a foreign key, remove the database level foreign key constraint it by setting `db_constraint=False`.
152+
- Remove the column and in the generated migration use `SafeRemoveField(..., deletion_action=DeletionAction.MOVE_TO_PENDING)` to replace `RemoveField(...)`. This only marks the state for the column as removed.
153+
- Combine these migrations together to save making multiple deploys
154+
- Deploy. It's important that all previous prs are in production before we remove the actual column from the table.
155+
- Make a PR that create a new migration that has the same `SafeRemoveField` operation as before, but set `deletion_action=DeletionAction.DELETE` instead. This deletes the actual column from the table in Postgres.
156+
- Deploy
157+
158+
Here's an example of removing the `project` column from this model. It is both a foreign key and not null:
159+
160+
```python
161+
@region_silo_model
162+
class TestModel(Model):
163+
__relocation_scope__ = RelocationScope.Excluded
164+
165+
project = FlexibleForeignKey("sentry.Project")
166+
name = models.TextField()
167+
168+
class Meta:
169+
app_label = "uptime"
170+
db_table = "uptime_testmodel"
171+
```
172+
173+
First we remove the constraint and make the column nullable:
174+
175+
```python
176+
# Model change
177+
...
178+
project = FlexibleForeignKey("sentry.Project", db_constraint=False, null=True)
179+
...
180+
181+
# Migration operations
182+
operations = [
183+
migrations.AlterField(
184+
model_name="testmodel",
185+
name="project",
186+
field=sentry.db.models.fields.foreignkey.FlexibleForeignKey(
187+
db_constraint=False,
188+
null=True,
189+
on_delete=django.db.models.deletion.CASCADE,
190+
to="sentry.project",
191+
),
192+
),
193+
]
194+
```
195+
196+
Once we've done this, we can now remove the column from the model and generate the migration to remove it. The generated migration looks like this:
197+
198+
```python
199+
operations = [
200+
migrations.RemoveField(model_name="testmodel", name="project"),
201+
]
202+
```
153203

154-
Here's an example of removing columns that were already nullable. First we remove the columns from the model, and then modify the migration to only update the state and make no database operations.
204+
Deleting this way is unsafe, so we want to replace this with `SafeDeleteModel` instead. So this becomes
155205

156206
```python
157207
operations = [
158-
migrations.SeparateDatabaseAndState(
159-
database_operations=[],
160-
state_operations=[
161-
migrations.RemoveField(model_name="alertrule", name="alert_threshold"),
162-
migrations.RemoveField(model_name="alertrule", name="resolve_threshold"),
163-
migrations.RemoveField(model_name="alertrule", name="threshold_type"),
164-
],
165-
)
208+
SafeRemoveField(model_name="testmodel", name="project", deletion_action=DeletionAction.MOVE_TO_PENDING),
166209
]
167210
```
168211

169-
Once this is deployed, we can then deploy the actual column deletion. This pr will have only a migration, since Django no longer knows about these fields. Note that the reverse SQL is only for dev, so it's fine to not assign a default or do any sort of backfill:
212+
Using `SafeRemoveField` allows us to remove all the state related to the column, but defer deleting it until a later migration. So now, we can combine the migration to remove the constraints and delete the column state together like so
170213

171214
```python
172215
operations = [
173-
migrations.SeparateDatabaseAndState(
174-
database_operations=[
175-
migrations.RunSQL(
176-
"""
177-
ALTER TABLE "sentry_alertrule" DROP COLUMN "alert_threshold";
178-
ALTER TABLE "sentry_alertrule" DROP COLUMN "resolve_threshold";
179-
ALTER TABLE "sentry_alertrule" DROP COLUMN "threshold_type";
180-
""",
181-
reverse_sql="""
182-
ALTER TABLE "sentry_alertrule" ADD COLUMN "alert_threshold" smallint NULL;
183-
ALTER TABLE "sentry_alertrule" ADD COLUMN "resolve_threshold" int NULL;
184-
ALTER TABLE "sentry_alertrule" ADD COLUMN "threshold_type" int NULL;
185-
""",
186-
hints={"tables": ["sentry_alertrule"]},
187-
)
188-
],
189-
state_operations=[],
190-
)
216+
migrations.AlterField(
217+
model_name="testmodel",
218+
name="project",
219+
field=sentry.db.models.fields.foreignkey.FlexibleForeignKey(
220+
db_constraint=False,
221+
null=True,
222+
on_delete=django.db.models.deletion.CASCADE,
223+
to="sentry.project",
224+
),
225+
),
226+
SafeRemoveField(model_name="testmodel", name="project", deletion_action=DeletionAction.MOVE_TO_PENDING),
227+
]
228+
```
229+
230+
So now we deploy this migration and move onto the final step.
231+
232+
In this last step, we will use `SafeRemoveField` again to finally remove the column from the table in Postgres. First, we generate an empty migration using `sentry django makemigrations <your_app> --empty` to produce an empty migration, and then modify the operations to be like:
233+
234+
```python
235+
operations = [
236+
SafeRemoveField(model_name="testmodel", name="project", deletion_action=DeletionAction.DELETE),
191237
]
192238
```
193239

240+
Then we deploy this and we're done. So to recap the steps here:
241+
- Remove all references to the column in the code in a separate PR and merge. Doesn't matter if this deploys before the next step or not.
242+
- If the column has an fk constraint them remove it. If it's not null and has no db_default then mark it as nullable. Then delete the column using SafeRemoveField(..., deletion_action=DeletionAction.MOVE_TO_PENDING). These operations can be in the same migration to save time.
243+
- Deploy all previous before continuing.
244+
- Remove the column from the table in from Postgres using SafeRemoveField(..., deletion_action=DeletionAction.DELETE),
245+
194246
### Deleting Tables
195247

196248
Extra care is needed here if the table is referenced as a foreign key in other tables. In that case, first remove the foreign key columns in the other tables, then come back to this step.
197249

198-
- Remove any database level foreign key constraints from this table to other tables by setting `db_constraint=False` on the columns.
199-
- Deploy
200-
- Remove the model and all references from the sentry codebase. Make sure that the migration only marks the state as removed.
201-
- Deploy.
202-
- Create a migrations that deletes the table.
250+
- (Optional, but ideal) Make a PR to remove all uses of the model in the codebase in a separate PR. This mostly helps with code cleanliness. This should be merged ahead of the migration prs, but we don't need to worry about whether it is deployed first
251+
- Make another PR to
252+
- Remove any database level foreign key constraints from this table to other tables by setting `db_constraint=False` on the columns.
253+
- Remove the model and in the generated migration use `SafeDeleteModel(..., deletion_action=DeletionAction.MOVE_TO_PENDING)` to replace `DeleteModel(...)`. This only marks the state for the model as removed.
254+
- Deploy. It's important that all previous prs are in production before we remove the actual table.
255+
- Make a PR that create a new migration that has the same `SafeDeleteModel` operation as before, but set `deletion_action=DeletionAction.DELETE` instead. This deletes the actual table from Postgres.
203256
- Deploy
204257

205258
Here's an example of removing this model:
206259

207260
```python
208-
class AlertRuleTriggerAction(Model):
209-
alert_rule_trigger = FlexibleForeignKey("sentry.AlertRuleTrigger")
210-
integration = FlexibleForeignKey("sentry.Integration", null=True)
211-
type = models.SmallIntegerField()
212-
target_type = models.SmallIntegerField()
213-
# Identifier used to perform the action on a given target
214-
target_identifier = models.TextField(null=True)
215-
# Human readable name to display in the UI
216-
target_display = models.TextField(null=True)
217-
date_added = models.DateTimeField(default=timezone.now)
261+
@region_silo_model
262+
class TestModel(Model):
263+
__relocation_scope__ = RelocationScope.Excluded
264+
265+
project = FlexibleForeignKey("sentry.Project")
266+
name = models.TextField()
218267

219268
class Meta:
220-
app_label = "sentry"
221-
db_table = "sentry_alertruletriggeraction"
269+
app_label = "uptime"
270+
db_table = "uptime_testmodel"
222271
```
223272

224-
First we checked that it's not referenced by any other models, and it's not. Next we need to remove and db level foreign key constraints. To do this, we change these two columns and generate a migration:
273+
First, we remove all references to this model from the codebase, including making sure that it's not referenced by any other models. This is best done as a separate pr to keep things clean.
274+
275+
Next we need to remove any db level foreign key constraints. To do this, we change this column and generate a migration:
225276

226277
```python
227-
alert_rule_trigger = FlexibleForeignKey("sentry.AlertRuleTrigger", db_constraint=False)
228-
integration = FlexibleForeignKey("sentry.Integration", null=True, db_constraint=False)
278+
project = FlexibleForeignKey("sentry.Project", db_constraint=False)
229279
```
230280

231281
The operations in the migration look like
232282

233283
```python
234284
operations = [
235285
migrations.AlterField(
236-
model_name='alertruletriggeraction',
237-
name='alert_rule_trigger',
238-
field=sentry.db.models.fields.foreignkey.FlexibleForeignKey(db_constraint=False, on_delete=django.db.models.deletion.CASCADE, to='sentry.AlertRuleTrigger'),
239-
),
240-
migrations.AlterField(
241-
model_name='alertruletriggeraction',
242-
name='integration',
243-
field=sentry.db.models.fields.foreignkey.FlexibleForeignKey(db_constraint=False, null=True, on_delete=django.db.models.deletion.CASCADE, to='sentry.Integration'),
286+
model_name="testmodel",
287+
name="project",
288+
field=sentry.db.models.fields.foreignkey.FlexibleForeignKey(
289+
db_constraint=False,
290+
on_delete=django.db.models.deletion.CASCADE,
291+
to="sentry.project",
292+
),
244293
),
245294
]
246295
```
247296

248-
And we can see the sql it generates just drops the FK constaints
297+
Once we've done this, we can now remove the model from code and generate the migration to remove it. The generated migration looks like this:
249298

250299
```python
251-
BEGIN;
252-
SET CONSTRAINTS "a875987ae7debe6be88869cb2eebcdc5" IMMEDIATE; ALTER TABLE "sentry_alertruletriggeraction" DROP CONSTRAINT "a875987ae7debe6be88869cb2eebcdc5";
253-
SET CONSTRAINTS "sentry_integration_id_14286d876e86361c_fk_sentry_integration_id" IMMEDIATE; ALTER TABLE "sentry_alertruletriggeraction" DROP CONSTRAINT "sentry_integration_id_14286d876e86361c_fk_sentry_integration_id";
254-
COMMIT;
300+
operations = [
301+
migrations.DeleteModel(name="TestModel"),
302+
]
255303
```
256304

257-
So now we deploy this and move onto the next stage.
305+
Deleting this way is unsafe, so we want to replace this with `SafeDeleteModel` instead. So this becomes
258306

259-
The next stage involves removing all references to the model from the codebase. So we do that, and then we generate a migration that removes the model from the migration state, but not the database. The operations in this migration look like
307+
```python
308+
operations = [
309+
SafeDeleteModel(name="TestModel", deletion_action=DeletionAction.MOVE_TO_PENDING),
310+
]
311+
```
260312

313+
Using `SafeDeleteModel` allows us to remove all the state related to the model, but defer deleting it until a later migration. So now, we can combine the migration to remove the constraints and delete the model state together like so
261314
```python
262315
operations = [
263-
migrations.SeparateDatabaseAndState(
264-
state_operations=[migrations.DeleteModel(name="AlertRuleTriggerAction")],
265-
database_operations=[],
266-
)
316+
migrations.AlterField(
317+
model_name="testmodel",
318+
name="project",
319+
field=sentry.db.models.fields.foreignkey.FlexibleForeignKey(
320+
db_constraint=False,
321+
on_delete=django.db.models.deletion.CASCADE,
322+
to="sentry.project",
323+
),
324+
),
325+
SafeDeleteModel(name="TestModel", deletion_action=DeletionAction.MOVE_TO_PENDING),
267326
]
268327
```
269328

270-
and the generated SQL shows no database changes occurring. So now we deploy this and move into the final step.
329+
So now we deploy this migration and move onto the final step.
271330

272-
In this last step, we just want to manually write DDL to remove the table. So we use `sentry django makemigrations --empty` to produce an empty migration, and then modify the operations to be like:
331+
In this last step, we will use `SafeDeleteModel` again to finally remove the table from Postgres. First, we generate an empty migration using `sentry django makemigrations <your_app> --empty` to produce an empty migration, and then modify the operations to be like:
273332

274333
```python
275334
operations = [
276-
migrations.RunSQL(
277-
"""
278-
DROP TABLE "sentry_alertruletriggeraction";
279-
""",
280-
reverse_sql="CREATE TABLE sentry_alertruletriggeraction (fake_col int)", # We just create a fake table here so that the DROP will work if we roll back the migration.
281-
)
335+
SafeDeleteModel(name="TestModel", deletion_action=DeletionAction.DELETE),
282336
]
283337
```
284338

285-
Then we deploy this and we're done.
339+
Then we deploy this and we're done. So to recap the steps here:
340+
- Remove all references to the model in the code in a separate PR and merge. Doesn't matter if this deploys before the next step or not.
341+
- Remove any fk constraints and delete the model using SafeDeleteModel(..., deletion_action=DeletionAction.MOVE_TO_PENDING). These operations can be in the same migration to save time.
342+
- Deploy all previous before continuing.
343+
- Remove the table from Postgres using SafeDeleteModel(..., deletion_action=DeletionAction.DELETE),
286344

287345
### Foreign Keys
288346

0 commit comments

Comments
 (0)