Skip to content

Commit 3e21e4d

Browse files
andrew-farriesandrew-farries
authored
Make backfill batch selection exclude rows inserted or updated after backfill start (#652)
Backfill only rows present at backfill start. This is third approach to solving #583. The previous two are: * #634 * #648 This is the most direct approach to solving the problem. At the same time as the up/down triggers are created to perform a backfill, a `_pgroll_needs_backfill` column is also created on the table to be backfilled. The column has a `DEFAULT` of `true`; the constant default ensures that this extra column can be added quickly without a lengthy `ACCESS_EXCLUSIVE` lock. The column is removed when the the operation is rolled back or completed. The up/down triggers are modified to set `_pgroll_needs_backfill` to false whenever they update a row. The backfill itself is updated to select only rows having `_pgroll_needs_backfill` set to `true` - this ensures that only rows created before the triggers were installed are updated by the backfill. The backfill process still needs to *read* every row in the table, including those inserted/updated after backfill start, but only those rows created before backfill start will be updated. The main disadvantage of this approach is that backfill now requires an extra column to be created on the target table.
1 parent 346f7f9 commit 3e21e4d

16 files changed

+235
-92
lines changed

‎docs/tutorial.md

Lines changed: 30 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -159,6 +159,8 @@ After some progress updates you should see a message saying that the migration h
159159
<summary>What's happening behind the progress updates?</summary>
160160

161161
In order to add the new `description` column, `pgroll` creates a temporary `_pgroll_new_description` column and copies over the data from the existing `description` column, using the `up` SQL from the migration. As we have 10^5 rows in our table, this process takes some time. This process is called _backfilling_ and it is performed in batches to avoid locking all rows in the table simultaneously.
162+
163+
The `_pgroll_needs_backfill` column in the `users` table is used to track which rows have been backfilled and which have not. This column is set to `true` for all rows at the start of the migration and set to `false` once the row has been backfilled. This ensures that rows inserted or updated while the backfill is in process aren't backfilled twice.
162164
</details>
163165

164166
At this point it's useful to look at the table data and schema to see what `pgroll` has done. Let's look at the data first:
@@ -169,19 +171,20 @@ SELECT * FROM users ORDER BY id LIMIT 10
169171

170172
You should see something like this:
171173
```
172-
+-----+----------+-------------------------+--------------------------+
173-
| id | name | description | _pgroll_new_description |
174-
+-----+----------+-------------------------+--------------------------+
175-
| 1 | user_1 | <null> | description for user_1 |
176-
| 2 | user_2 | description for user_2 | description for user_2 |
177-
| 3 | user_3 | <null> | description for user_3 |
178-
| 4 | user_4 | description for user_4 | description for user_4 |
179-
| 5 | user_5 | <null> | description for user_5 |
180-
| 6 | user_6 | description for user_6 | description for user_6 |
181-
| 7 | user_7 | <null> | description for user_7 |
182-
| 8 | user_8 | <null> | description for user_8 |
183-
| 9 | user_9 | description for user_9 | description for user_9 |
184-
| 10 | user_10 | description for user_10 | description for user_10 |
174+
+----+---------+------------------------+-------------------------+------------------------+
175+
| id | name | description | _pgroll_new_description | _pgroll_needs_backfill |
176+
|----+---------+------------------------+-------------------------+------------------------|
177+
| 1 | user_1 | <null> | description for user_1 | False |
178+
| 2 | user_2 | description for user_2 | description for user_2 | False |
179+
| 3 | user_3 | <null> | description for user_3 | False |
180+
| 4 | user_4 | <null> | description for user_4 | False |
181+
| 5 | user_5 | <null> | description for user_5 | False |
182+
| 6 | user_6 | description for user_6 | description for user_6 | False |
183+
| 7 | user_7 | <null> | description for user_7 | False |
184+
| 8 | user_8 | description for user_8 | description for user_8 | False |
185+
| 9 | user_9 | <null> | description for user_9 | False |
186+
| 10 | user_10 | <null> | description for user_10 | False |
187+
+----+---------+------------------------+-------------------------+------------------------+
185188
```
186189

187190
This is the "expand" phase of the [expand/contract pattern](https://openpracticelibrary.com/practice/expand-and-contract-pattern/) in action; `pgroll` has added a `_pgroll_new_description` field to the table and populated the field for all rows using the `up` SQL from the `02_user_description_set_nullable.json` file:
@@ -201,21 +204,22 @@ DESCRIBE users
201204
You should see something like this:
202205

203206
```
204-
+-------------------------+------------------------+-----------------------------------------------------------------+
205-
| Column | Type | Modifiers |
206-
+-------------------------+------------------------+-----------------------------------------------------------------+
207-
| id | integer | not null default nextval('_pgroll_new_users_id_seq'::regclass) |
208-
| name | character varying(255) | not null |
209-
| description | text | |
210-
| _pgroll_new_description | text | |
211-
+-------------------------+------------------------+-----------------------------------------------------------------+
207+
+-------------------------+------------------------+-----------------------------------------------------+
208+
| Column | Type | Modifiers |
209+
|-------------------------+------------------------+-----------------------------------------------------|
210+
| id | integer | not null default nextval('users_id_seq'::regclass) |
211+
| name | character varying(255) | not null |
212+
| description | text | |
213+
| _pgroll_new_description | text | |
214+
| _pgroll_needs_backfill | boolean | default true |
215+
+-------------------------+------------------------+-----------------------------------------------------+
212216
Indexes:
213-
"_pgroll_new_users_pkey" PRIMARY KEY, btree (id)
214-
"_pgroll_new_users_name_key" UNIQUE CONSTRAINT, btree (name)
217+
"users_pkey" PRIMARY KEY, btree (id)
218+
"users_name_key" UNIQUE CONSTRAINT, btree (name)
215219
Check constraints:
216-
"_pgroll_add_column_check_description" CHECK (_pgroll_new_description IS NOT NULL) NOT VALID
220+
"_pgroll_check_not_null_description" CHECK (_pgroll_new_description IS NOT NULL) NOT VALID
217221
Triggers:
218-
_pgroll_trigger_users__pgroll_new_description BEFORE INSERT OR UPDATE ON users FOR EACH ROW EXECUTE FUNCTION _pgroll_trigger_users__pgroll_new_description>
222+
_pgroll_trigger_users__pgroll_new_description BEFORE INSERT OR UPDATE ON users FOR EACH ROW EXECUTE FUNCTION _pgroll_trigger_users__pgroll_new_description()
219223
_pgroll_trigger_users_description BEFORE INSERT OR UPDATE ON users FOR EACH ROW EXECUTE FUNCTION _pgroll_trigger_users_description()
220224
```
221225

@@ -411,6 +415,7 @@ Indexes:
411415
A few things have happened:
412416

413417
- The extra `_pgroll_new_description` has been renamed to `description`.
418+
- The `_pgroll_needs_backfill` column has been removed.
414419
- The old `description` column has been removed.
415420
- The `description` column is now marked as `NOT NULL`.
416421
- The triggers to copy data back and forth between the old and new column have been removed.

‎pkg/backfill/backfill.go

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -53,9 +53,10 @@ func (bf *Backfill) Start(ctx context.Context, table *schema.Table) error {
5353
// Create a batcher for the table.
5454
b := batcher{
5555
BatchConfig: templates.BatchConfig{
56-
TableName: table.Name,
57-
PrimaryKey: identityColumns,
58-
BatchSize: bf.batchSize,
56+
TableName: table.Name,
57+
PrimaryKey: identityColumns,
58+
BatchSize: bf.batchSize,
59+
NeedsBackfillColumn: "_pgroll_needs_backfill",
5960
},
6061
}
6162

‎pkg/backfill/templates/build.go

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -11,10 +11,11 @@ import (
1111
)
1212

1313
type BatchConfig struct {
14-
TableName string
15-
PrimaryKey []string
16-
LastValue []string
17-
BatchSize int
14+
TableName string
15+
PrimaryKey []string
16+
LastValue []string
17+
BatchSize int
18+
NeedsBackfillColumn string
1819
}
1920

2021
func BuildSQL(cfg BatchConfig) (string, error) {

‎pkg/backfill/templates/build_test.go

Lines changed: 24 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -15,35 +15,39 @@ func TestBatchStatementBuilder(t *testing.T) {
1515
}{
1616
"single identity column no last value": {
1717
config: BatchConfig{
18-
TableName: "table_name",
19-
PrimaryKey: []string{"id"},
20-
BatchSize: 10,
18+
TableName: "table_name",
19+
PrimaryKey: []string{"id"},
20+
NeedsBackfillColumn: "_pgroll_needs_backfill",
21+
BatchSize: 10,
2122
},
2223
expected: expectSingleIDColumnNoLastValue,
2324
},
2425
"multiple identity columns no last value": {
2526
config: BatchConfig{
26-
TableName: "table_name",
27-
PrimaryKey: []string{"id", "zip"},
28-
BatchSize: 10,
27+
TableName: "table_name",
28+
PrimaryKey: []string{"id", "zip"},
29+
NeedsBackfillColumn: "_pgroll_needs_backfill",
30+
BatchSize: 10,
2931
},
3032
expected: multipleIDColumnsNoLastValue,
3133
},
3234
"single identity column with last value": {
3335
config: BatchConfig{
34-
TableName: "table_name",
35-
PrimaryKey: []string{"id"},
36-
LastValue: []string{"1"},
37-
BatchSize: 10,
36+
TableName: "table_name",
37+
PrimaryKey: []string{"id"},
38+
NeedsBackfillColumn: "_pgroll_needs_backfill",
39+
LastValue: []string{"1"},
40+
BatchSize: 10,
3841
},
3942
expected: singleIDColumnWithLastValue,
4043
},
4144
"multiple identity columns with last value": {
4245
config: BatchConfig{
43-
TableName: "table_name",
44-
PrimaryKey: []string{"id", "zip"},
45-
LastValue: []string{"1", "1234"},
46-
BatchSize: 10,
46+
TableName: "table_name",
47+
PrimaryKey: []string{"id", "zip"},
48+
NeedsBackfillColumn: "_pgroll_needs_backfill",
49+
LastValue: []string{"1", "1234"},
50+
BatchSize: 10,
4751
},
4852
expected: multipleIDColumnsWithLastValue,
4953
},
@@ -63,6 +67,7 @@ const expectSingleIDColumnNoLastValue = `WITH batch AS
6367
(
6468
SELECT "id"
6569
FROM "table_name"
70+
WHERE "_pgroll_needs_backfill" = true
6671
ORDER BY "id"
6772
LIMIT 10
6873
FOR NO KEY UPDATE
@@ -83,6 +88,7 @@ const multipleIDColumnsNoLastValue = `WITH batch AS
8388
(
8489
SELECT "id", "zip"
8590
FROM "table_name"
91+
WHERE "_pgroll_needs_backfill" = true
8692
ORDER BY "id", "zip"
8793
LIMIT 10
8894
FOR NO KEY UPDATE
@@ -103,7 +109,8 @@ const singleIDColumnWithLastValue = `WITH batch AS
103109
(
104110
SELECT "id"
105111
FROM "table_name"
106-
WHERE ("id") > ('1')
112+
WHERE "_pgroll_needs_backfill" = true
113+
AND ("id") > ('1')
107114
ORDER BY "id"
108115
LIMIT 10
109116
FOR NO KEY UPDATE
@@ -124,7 +131,8 @@ const multipleIDColumnsWithLastValue = `WITH batch AS
124131
(
125132
SELECT "id", "zip"
126133
FROM "table_name"
127-
WHERE ("id", "zip") > ('1', '1234')
134+
WHERE "_pgroll_needs_backfill" = true
135+
AND ("id", "zip") > ('1', '1234')
128136
ORDER BY "id", "zip"
129137
LIMIT 10
130138
FOR NO KEY UPDATE

‎pkg/backfill/templates/sql.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,9 @@ const SQL = `WITH batch AS
66
(
77
SELECT {{ commaSeparate (quoteIdentifiers .PrimaryKey) }}
88
FROM {{ .TableName | qi}}
9+
WHERE {{ .NeedsBackfillColumn | qi }} = true
910
{{ if .LastValue -}}
10-
WHERE ({{ commaSeparate (quoteIdentifiers .PrimaryKey) }}) > ({{ commaSeparate (quoteLiterals .LastValue) }})
11+
AND ({{ commaSeparate (quoteIdentifiers .PrimaryKey) }}) > ({{ commaSeparate (quoteLiterals .LastValue) }})
1112
{{ end -}}
1213
ORDER BY {{ commaSeparate (quoteIdentifiers .PrimaryKey) }}
1314
LIMIT {{ .BatchSize }}

‎pkg/migrations/op_add_column.go

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,14 @@ func (o *OpAddColumn) Complete(ctx context.Context, conn db.DB, tr SQLTransforme
9494
return err
9595
}
9696

97+
// Remove the needs backfill column
98+
_, err = conn.ExecContext(ctx, fmt.Sprintf("ALTER TABLE IF EXISTS %s DROP COLUMN IF EXISTS %s",
99+
pq.QuoteIdentifier(o.Table),
100+
pq.QuoteIdentifier(CNeedsBackfillColumn)))
101+
if err != nil {
102+
return err
103+
}
104+
97105
if !o.Column.IsNullable() && o.Column.Default == nil {
98106
_, err = conn.ExecContext(ctx, fmt.Sprintf("ALTER TABLE IF EXISTS %s VALIDATE CONSTRAINT %s",
99107
pq.QuoteIdentifier(o.Table),
@@ -169,6 +177,15 @@ func (o *OpAddColumn) Rollback(ctx context.Context, conn db.DB, tr SQLTransforme
169177

170178
_, err = conn.ExecContext(ctx, fmt.Sprintf("DROP FUNCTION IF EXISTS %s CASCADE",
171179
pq.QuoteIdentifier(TriggerFunctionName(o.Table, o.Column.Name))))
180+
if err != nil {
181+
return err
182+
}
183+
184+
// Remove the needs backfill column
185+
_, err = conn.ExecContext(ctx, fmt.Sprintf("ALTER TABLE IF EXISTS %s DROP COLUMN IF EXISTS %s",
186+
pq.QuoteIdentifier(table.Name),
187+
pq.QuoteIdentifier(CNeedsBackfillColumn)))
188+
172189
return err
173190
}
174191

‎pkg/migrations/op_add_column_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1127,9 +1127,9 @@ func TestAddColumnWithUpSql(t *testing.T) {
11271127
// after rollback + restart + complete, all 'description' values are the backfilled ones.
11281128
res := MustSelect(t, db, schema, "02_add_column", "products")
11291129
assert.Equal(t, []map[string]any{
1130+
{"id": "c", "name": "cherries", "description": "CHERRIES"},
11301131
{"id": "a", "name": "apple", "description": "APPLE"},
11311132
{"id": "b", "name": "banana", "description": "BANANA"},
1132-
{"id": "c", "name": "cherries", "description": "CHERRIES"},
11331133
}, res)
11341134

11351135
// The trigger function has been dropped.

‎pkg/migrations/op_alter_column.go

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -118,6 +118,14 @@ func (o *OpAlterColumn) Complete(ctx context.Context, conn db.DB, tr SQLTransfor
118118
return err
119119
}
120120

121+
// Remove the needs backfill column
122+
_, err = conn.ExecContext(ctx, fmt.Sprintf("ALTER TABLE IF EXISTS %s DROP COLUMN IF EXISTS %s",
123+
pq.QuoteIdentifier(o.Table),
124+
pq.QuoteIdentifier(CNeedsBackfillColumn)))
125+
if err != nil {
126+
return err
127+
}
128+
121129
// Rename the new column to the old column name
122130
table := s.GetTable(o.Table)
123131
if table == nil {
@@ -177,7 +185,12 @@ func (o *OpAlterColumn) Rollback(ctx context.Context, conn db.DB, tr SQLTransfor
177185
return err
178186
}
179187

180-
return nil
188+
// Remove the needs backfill column
189+
_, err = conn.ExecContext(ctx, fmt.Sprintf("ALTER TABLE IF EXISTS %s DROP COLUMN IF EXISTS %s",
190+
pq.QuoteIdentifier(table.Name),
191+
pq.QuoteIdentifier(CNeedsBackfillColumn)))
192+
193+
return err
181194
}
182195

183196
func (o *OpAlterColumn) Validate(ctx context.Context, s *schema.Schema) error {

‎pkg/migrations/op_common_test.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -882,13 +882,17 @@ func MustSelect(t *testing.T, db *sql.DB, schema, version, table string) []map[s
882882
// - The down functions for the columns no longer exist.
883883
// - The up triggers for the columns no longer exist.
884884
// - The down triggers for the columns no longer exist.
885+
// - The _pgroll_needs_backfill column should not exist on the table.
885886
func TableMustBeCleanedUp(t *testing.T, db *sql.DB, schema, table string, columns ...string) {
886887
t.Helper()
887888

888889
for _, column := range columns {
889890
// The temporary column should not exist on the underlying table.
890891
ColumnMustNotExist(t, db, schema, table, migrations.TemporaryName(column))
891892

893+
// The _pgroll_needs_backfill column should not exist on the table.
894+
ColumnMustNotExist(t, db, schema, table, migrations.CNeedsBackfillColumn)
895+
892896
// The up function for the column no longer exists.
893897
FunctionMustNotExist(t, db, schema, migrations.TriggerFunctionName(table, column))
894898
// The down function for the column no longer exists.

‎pkg/migrations/op_create_constraint.go

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -157,7 +157,16 @@ func (o *OpCreateConstraint) Complete(ctx context.Context, conn db.DB, tr SQLTra
157157
}
158158
}
159159

160-
return o.removeTriggers(ctx, conn)
160+
if err := o.removeTriggers(ctx, conn); err != nil {
161+
return err
162+
}
163+
164+
// Remove the needs backfill column
165+
_, err = conn.ExecContext(ctx, fmt.Sprintf("ALTER TABLE IF EXISTS %s DROP COLUMN IF EXISTS %s",
166+
pq.QuoteIdentifier(o.Table),
167+
pq.QuoteIdentifier(CNeedsBackfillColumn)))
168+
169+
return err
161170
}
162171

163172
func (o *OpCreateConstraint) Rollback(ctx context.Context, conn db.DB, tr SQLTransformer, s *schema.Schema) error {
@@ -174,7 +183,16 @@ func (o *OpCreateConstraint) Rollback(ctx context.Context, conn db.DB, tr SQLTra
174183
return err
175184
}
176185

177-
return o.removeTriggers(ctx, conn)
186+
if err := o.removeTriggers(ctx, conn); err != nil {
187+
return err
188+
}
189+
190+
// Remove the needs backfill column
191+
_, err = conn.ExecContext(ctx, fmt.Sprintf("ALTER TABLE IF EXISTS %s DROP COLUMN IF EXISTS %s",
192+
pq.QuoteIdentifier(table.Name),
193+
pq.QuoteIdentifier(CNeedsBackfillColumn)))
194+
195+
return err
178196
}
179197

180198
func (o *OpCreateConstraint) removeTriggers(ctx context.Context, conn db.DB) error {

0 commit comments

Comments
 (0)