summaryrefslogtreecommitdiff
path: root/doc/development/database/loose_foreign_keys.md
blob: 17a825b4812312c229e0c4833fe939f047fb6b68 (plain)
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
153
154
155
156
157
158
159
160
161
162
163
164
165
166
167
168
169
170
171
172
173
174
175
176
177
178
179
180
181
182
183
184
185
186
187
188
189
190
191
192
193
194
195
196
197
198
199
200
201
202
203
204
205
206
207
208
209
210
211
212
213
214
215
216
217
218
219
220
221
222
223
224
225
226
227
228
229
230
231
232
233
234
235
236
237
238
239
240
241
242
243
244
245
246
247
248
249
250
251
252
253
254
255
256
257
258
259
260
261
262
263
264
265
266
267
268
269
270
271
272
273
274
275
276
277
278
279
280
281
282
283
284
285
286
287
288
289
290
291
292
293
294
295
296
297
298
299
300
301
302
303
304
305
306
307
308
309
310
311
312
313
314
315
316
317
318
319
320
321
322
323
324
325
326
327
328
329
330
331
332
333
334
335
336
337
338
339
340
341
342
343
344
345
346
347
348
349
350
351
352
353
354
355
356
357
358
359
360
361
362
363
364
365
366
367
368
369
370
371
372
373
374
375
376
377
378
379
380
381
382
383
384
385
386
387
388
389
390
391
392
393
394
395
396
397
398
399
400
401
402
403
404
405
406
407
408
409
410
411
412
413
414
415
416
417
418
419
420
421
422
423
424
425
426
427
428
429
430
431
432
433
434
435
436
437
438
439
440
441
442
443
444
445
446
447
448
449
450
451
452
453
454
455
456
457
458
459
460
461
462
463
464
465
466
467
468
469
470
471
472
473
474
475
476
477
478
479
480
481
482
---
stage: Enablement
group: Database
info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/engineering/ux/technical-writing/#assignments
---

# Loose foreign keys

## Problem statement

In relational databases (including PostgreSQL), foreign keys provide a way to link
two database tables together, and ensure data-consistency between them. In GitLab,
[foreign keys](../foreign_keys.md) are vital part of the database design process.
Most of our database tables have foreign keys.

With the ongoing database [decomposition work](https://gitlab.com/groups/gitlab-org/-/epics/6168),
linked records might be present on two different database servers. Ensuring data consistency
between two databases is not possible with standard PostgreSQL foreign keys. PostgreSQL
does not support foreign keys operating within a single database server, defining
a link between two database tables in two different database servers over the network.

Example:

- Database "Main": `projects` table
- Database "CI": `ci_pipelines` table

A project can have many pipelines. When a project is deleted, the associated `ci_pipeline` (via the
`project_id` column) records must be also deleted.

With a multi-database setup, this cannot be achieved with foreign keys.

## Asynchronous approach

Our preferred approach to this problem is eventual consistency. With the loose foreign keys
feature, we can configure delayed association cleanup without negatively affecting the
application performance.

### How it works

In the previous example, a record in the `projects` table can have multiple `ci_pipeline`
records. To keep the cleanup process separate from the actual parent record deletion,
we can:

1. Create a `DELETE` trigger on the `projects` table.
   Record the deletions in a separate table (`deleted_records`).
1. A job checks the `deleted_records` table every minute or two.
1. For each record in the table, delete the associated `ci_pipelines` records
   using the `project_id` column.

NOTE:
For this procedure to work, we must register which tables to clean up asynchronously.

## The `scripts/decomposition/generate-loose-foreign-key`

We built an automation tool to aid migration of foreign keys into loose foreign keys as part of
decomposition effort. It presents existing keys and allows chosen foreign keys to be automatically
converted into loose foreign keys. This ensures consistency between foreign key and loose foreign
key definitions, and ensures that they are properly tested.

WARNING:
We strongly advise you to use the automation script for swapping any foreign key to a loose foreign key.

The tool ensures that all aspects of swapping a foreign key are covered. This includes:

- Creating a migration to remove a foreign key.
- Updating `db/structure.sql` with the new migration.
- Updating `lib/gitlab/database/gitlab_loose_foreign_keys.yml` to add the new loose foreign key.
- Creating or updating a model's specs to ensure that the loose foreign key is properly supported.
- Creating a new branch, commit, push, and creating a merge request on GitLab.com.
- Creating a merge request template with all the necessary details to validate the safety of the foreign key removal.

The tool is located at `scripts/decomposition/generate-loose-foreign-key`:

```shell
$ scripts/decomposition/generate-loose-foreign-key -h

Usage: scripts/decomposition/generate-loose-foreign-key [options] <filters...>
    -c, --cross-schema               Show only cross-schema foreign keys
    -n, --dry-run                    Do not execute any commands (dry run)
    -b, --[no-]branch                Create or not a new branch
    -r, --[no-]rspec                 Create or not a rspecs automatically
    -m, --milestone MILESTONE        Specify custom milestone (current: 14.8)
    -h, --help                       Prints this help
```

For the migration of cross-schema foreign keys, we use the `-c` modifier to show the foreign keys
yet to migrate:

```shell
$ scripts/decomposition/generate-loose-foreign-key -c
Re-creating current test database
Dropped database 'gitlabhq_test_ee'
Dropped database 'gitlabhq_geo_test_ee'
Created database 'gitlabhq_test_ee'
Created database 'gitlabhq_geo_test_ee'

Showing cross-schema foreign keys (20):
   ID | HAS_LFK |                                     FROM |                   TO |                         COLUMN |       ON_DELETE 
    0 |       N |                                ci_builds |             projects |                     project_id |         cascade 
    1 |       N |                         ci_job_artifacts |             projects |                     project_id |         cascade 
    2 |       N |                             ci_pipelines |             projects |                     project_id |         cascade 
    3 |       Y |                             ci_pipelines |       merge_requests |               merge_request_id |         cascade 
    4 |       N |                   external_pull_requests |             projects |                     project_id |         cascade 
    5 |       N |                     ci_sources_pipelines |             projects |                     project_id |         cascade 
    6 |       N |                                ci_stages |             projects |                     project_id |         cascade 
    7 |       N |                    ci_pipeline_schedules |             projects |                     project_id |         cascade 
    8 |       N |                       ci_runner_projects |             projects |                     project_id |         cascade 
    9 |       Y |             dast_site_profiles_pipelines |         ci_pipelines |                 ci_pipeline_id |         cascade 
   10 |       Y |                   vulnerability_feedback |         ci_pipelines |                    pipeline_id |         nullify 
   11 |       N |                             ci_variables |             projects |                     project_id |         cascade 
   12 |       N |                                  ci_refs |             projects |                     project_id |         cascade 
   13 |       N |                       ci_builds_metadata |             projects |                     project_id |         cascade 
   14 |       N |                ci_subscriptions_projects |             projects |          downstream_project_id |         cascade 
   15 |       N |                ci_subscriptions_projects |             projects |            upstream_project_id |         cascade 
   16 |       N |                      ci_sources_projects |             projects |              source_project_id |         cascade 
   17 |       N |         ci_job_token_project_scope_links |             projects |              source_project_id |         cascade 
   18 |       N |         ci_job_token_project_scope_links |             projects |              target_project_id |         cascade 
   19 |       N |                ci_project_monthly_usages |             projects |                     project_id |         cascade 

To match FK write one or many filters to match against FROM/TO/COLUMN:
- scripts/decomposition/generate-loose-foreign-key <filter(s)...>
- scripts/decomposition/generate-loose-foreign-key ci_job_artifacts project_id
- scripts/decomposition/generate-loose-foreign-key dast_site_profiles_pipelines
```

The command accepts a list of filters to match from, to, or column for the purpose of the foreign key generation.
For example, run this to swap all foreign keys for `ci_job_token_project_scope_links` for the
decomposed database:

```shell
scripts/decomposition/generate-loose-foreign-key -c ci_job_token_project_scope_links
```

To swap only the `source_project_id` of `ci_job_token_project_scope_links` for the decomposed database, run:

```shell
scripts/decomposition/generate-loose-foreign-key -c ci_job_token_project_scope_links source_project_id
```

To swap all the foreign keys (all having `_id` appended), but not create a new branch (only commit
the changes) and not create rspecs, run:

```shell
scripts/decomposition/generate-loose-foreign-key -c --no-branch --no-rspec _id
```

To swap all foreign keys referencing `projects`, but not create a new branch (only commit the
changes), run:

```shell
scripts/decomposition/generate-loose-foreign-key -c --no-branch projects
```

## Example migration and configuration

### Configure the loose foreign key

Loose foreign keys are defined in a YAML file. The configuration requires the
following information:

- Parent table name (`projects`)
- Child table name (`ci_pipelines`)
- The data cleanup method (`async_delete` or `async_nullify`)

The YAML file is located at `config/gitlab_loose_foreign_keys.yml`. The file groups
foreign key definitions by the name of the child table. The child table can have multiple loose
foreign key definitions, therefore we store them as an array.

Example definition:

```yaml
ci_pipelines:
  - table: projects
    column: project_id
    on_delete: async_delete
```

If the `ci_pipelines` key is already present in the YAML file, then a new entry can be added
to the array:

```yaml
ci_pipelines:
  - table: projects
    column: project_id
    on_delete: async_delete
  - table: another_table
    column: another_id
    on_delete: :async_nullify
```

### Track record changes

To know about deletions in the `projects` table, configure a `DELETE` trigger
using a [post-deployment migration](../post_deployment_migrations.md). The
trigger needs to be configured only once. If the model already has at least one
`loose_foreign_key` definition, then this step can be skipped:

```ruby
class TrackProjectRecordChanges < Gitlab::Database::Migration[1.0]
  include Gitlab::Database::MigrationHelpers::LooseForeignKeyHelpers

  enable_lock_retries!

  def up
    track_record_deletions(:projects)
  end

  def down
    untrack_record_deletions(:projects)
  end
end
```

### Remove the foreign key

If there is an existing foreign key, then it can be removed from the database. As of GitLab 14.5,
the following foreign key describes the link between the `projects` and `ci_pipelines` tables:

```sql
ALTER TABLE ONLY ci_pipelines
ADD CONSTRAINT fk_86635dbd80
FOREIGN KEY (project_id)
REFERENCES projects(id)
ON DELETE CASCADE;
```

The migration must run after the `DELETE` trigger is installed and the loose
foreign key definition is deployed. As such, it must be a [post-deployment
migration](../post_deployment_migrations.md) dated after the migration for the
trigger. If the foreign key is deleted earlier, there is a good chance of
introducing data inconsistency which needs manual cleanup:

```ruby
class RemoveProjectsCiPipelineFk < Gitlab::Database::Migration[1.0]
  disable_ddl_transaction!

  def up
    with_lock_retries do
      remove_foreign_key_if_exists(:ci_pipelines, :projects, name: "fk_86635dbd80")
    end
  end

  def down
    add_concurrent_foreign_key(:ci_pipelines, :projects, name: "fk_86635dbd80", column: :project_id, target_column: :id, on_delete: "cascade")
  end
end
```

At this point, the setup phase is concluded. The deleted `projects` records should be automatically
picked up by the scheduled cleanup worker job.

### Remove the loose foreign key

When the loose foreign key definition is no longer needed (parent table is removed, or FK is restored),
we need to remove the definition from the YAML file and ensure that we don't leave pending deleted
records in the database.

1. Remove the loose foreign key definition from the config (`config/gitlab_loose_foreign_keys.yml`).
1. Remove the deletion tracking trigger from the parent table (if the parent table is still there).
1. Remove leftover deleted records from the `loose_foreign_keys_deleted_records` table.

Migration for removing the trigger:

```ruby
class UnTrackProjectRecordChanges < Gitlab::Database::Migration[1.0]
  include Gitlab::Database::MigrationHelpers::LooseForeignKeyHelpers

  enable_lock_retries!

  def up
    untrack_record_deletions(:projects)
  end

  def down
    track_record_deletions(:projects)
  end
end
```

With the trigger removal, we prevent further records to be inserted in the `loose_foreign_keys_deleted_records`
table however, there is still a chance for having leftover pending records in the table. These records
must be removed with an inline data migration.

```ruby
class RemoveLeftoverProjectDeletions < Gitlab::Database::Migration[1.0]
  disable_ddl_transaction!

  def up
    loop do
      result = execute <<~SQL
      DELETE FROM "loose_foreign_keys_deleted_records"
      WHERE
      ("loose_foreign_keys_deleted_records"."partition", "loose_foreign_keys_deleted_records"."id") IN (
        SELECT "loose_foreign_keys_deleted_records"."partition", "loose_foreign_keys_deleted_records"."id"
        FROM "loose_foreign_keys_deleted_records"
        WHERE
        "loose_foreign_keys_deleted_records"."fully_qualified_table_name" = 'public.projects' AND
        "loose_foreign_keys_deleted_records"."status" = 1
        LIMIT 100
      )
      SQL

      break if result.cmd_tuples == 0
    end
  end

  def down
    # no-op
  end
end
```

## Testing

The "`it has loose foreign keys`" shared example can be used to test the presence of the `ON DELETE` trigger and the
loose foreign key definitions.

Simply add to the model test file:

```ruby
it_behaves_like 'it has loose foreign keys' do
  let(:factory_name) { :project }
end
```

**After** [removing a foreign key](#remove-the-foreign-key),
use the "`cleanup by a loose foreign key`" shared example to test a child record's deletion or nullification
via the added loose foreign key:

```ruby
it_behaves_like 'cleanup by a loose foreign key' do
  let!(:model) { create(:ci_pipeline, user: create(:user)) }
  let!(:parent) { model.user }
end
```

## Caveats of loose foreign keys

### Record creation

The feature provides an efficient way of cleaning up associated records after the parent record is
deleted. Without foreign keys, it's the application's responsibility to validate if the parent record
exists when a new associated record is created.

A bad example: record creation with the given ID (`project_id` comes from user input).
In this example, nothing prevents us from passing a random project ID:

```ruby
Ci::Pipeline.create!(project_id: params[:project_id])
```

A good example: record creation with extra check:

```ruby
project = Project.find(params[:project_id])
Ci::Pipeline.create!(project_id: project.id)
```

### Association lookup

Consider the following HTTP request:

```plaintext
GET /projects/5/pipelines/100
```

The controller action ignores the `project_id` parameter and finds the pipeline using the ID:

```ruby
  def show
  # bad, avoid it
  pipeline = Ci::Pipeline.find(params[:id]) # 100
end
```

This endpoint still works when the parent `Project` model is deleted. This can be considered a
a data leak which should not happen under normal circumstances:

```ruby
def show
  # good
  project = Project.find(params[:project_id])
  pipeline = project.pipelines.find(params[:pipeline_id]) # 100
end
```

NOTE:
This example is unlikely in GitLab, because we usually look up the parent models to perform
permission checks.

## A note on `dependent: :destroy` and `dependent: :nullify`

We considered using these Rails features as an alternative to foreign keys but there are several problems which include:

1. These run on a different connection in the context of a transaction [which we do not allow](multiple_databases.md#removing-cross-database-transactions).
1. These can lead to severe performance degradation as we load all records from PostgreSQL, loop over them in Ruby, and call individual `DELETE` queries.
1. These can miss data as they only cover the case when the `destroy` method is called directly on the model. There are other cases including `delete_all` and cascading deletes from another parent table that could mean these are missed.

For non-trivial objects that need to clean up data outside the
database (for example, object storage) where you might wish to use `dependent: :destroy`,
see alternatives in
[Avoid `dependent: :nullify` and `dependent: :destroy` across
databases](./multiple_databases.md#avoid-dependent-nullify-and-dependent-destroy-across-databases).

## Risks of loose foreign keys and possible mitigations

In general, the loose foreign keys architecture is eventually consistent and
the cleanup latency might lead to problems visible to GitLab users or
operators. We consider the tradeoff as acceptable, but there might be
cases where the problems are too frequent or too severe, and we must
implement a mitigation strategy. A general mitigation strategy might be to have
an "urgent" queue for cleanup of records that have higher impact with a delayed
cleanup.

Below are some more specific examples of problems that might occur and how we
might mitigate them. In all the listed cases we might still consider the problem
described to be low risk and low impact, and in that case we would choose to not
implement any mitigation.

### The record should be deleted but it shows up in a view

This hypothetical example might happen with a foreign key like:

```sql
ALTER TABLE ONLY vulnerability_occurrence_pipelines
    ADD CONSTRAINT fk_rails_6421e35d7d FOREIGN KEY (pipeline_id) REFERENCES ci_pipelines(id) ON DELETE CASCADE;
```

In this example we expect to delete all associated `vulnerability_occurrence_pipelines` records
whenever we delete the `ci_pipelines` record associated with them. In this case
you might end up with some vulnerability page in GitLab which shows an occurrence
of a vulnerability. However, when you try to click a link to the pipeline, you get
a 404, because the pipeline is deleted. Then, when you navigate back you might find the
occurrence has disappeared too.

**Mitigation**

When rendering the vulnerability occurrences on the vulnerability page we could
try to load the corresponding pipeline and choose to skip displaying that
occurrence if pipeline is not found.

### The deleted parent record is needed to render a view and causes a `500` error

This hypothetical example might happen with a foreign key like:

```sql
ALTER TABLE ONLY vulnerability_occurrence_pipelines
    ADD CONSTRAINT fk_rails_6421e35d7d FOREIGN KEY (pipeline_id) REFERENCES ci_pipelines(id) ON DELETE CASCADE;
```

In this example we expect to delete all associated `vulnerability_occurrence_pipelines` records
whenever we delete the `ci_pipelines` record associated with them. In this case
you might end up with a vulnerability page in GitLab which shows an "occurrence"
of a vulnerability. However, when rendering the occurrence we try to load, for example,
`occurrence.pipeline.created_at`, which causes a 500 for the user.

**Mitigation**

When rendering the vulnerability occurrences on the vulnerability page we could
try to load the corresponding pipeline and choose to skip displaying that
occurrence if pipeline is not found.

### The deleted parent record is accessed in a Sidekiq worker and causes a failed job

This hypothetical example might happen with a foreign key like:

```sql
ALTER TABLE ONLY vulnerability_occurrence_pipelines
    ADD CONSTRAINT fk_rails_6421e35d7d FOREIGN KEY (pipeline_id) REFERENCES ci_pipelines(id) ON DELETE CASCADE;
```

In this example we expect to delete all associated `vulnerability_occurrence_pipelines` records
whenever we delete the `ci_pipelines` record associated with them. In this case
you might end up with a Sidekiq worker that is responsible for processing a
vulnerability and looping over all occurrences causing a Sidekiq job to fail if
it executes `occurrence.pipeline.created_at`.

**Mitigation**

When looping through the vulnerability occurrences in the Sidekiq worker, we
could try to load the corresponding pipeline and choose to skip processing that
occurrence if pipeline is not found.