Skip to content

Commit 9acea52

Browse files
michaelpqmichaelpq
committed
Fix corruption of toast indexes with REINDEX CONCURRENTLY
REINDEX CONCURRENTLY run on a toast index or a toast relation could corrupt the target indexes rebuilt, as a backend running in parallel that manipulates toast values would directly release the lock on the toast relation when its local operation is done, rather than releasing the lock once the transaction that manipulated the toast values committed. The fix done here is simple: we now hold a ROW EXCLUSIVE lock on the toast relation when saving or deleting a toast value until the transaction working on them is committed, so as a concurrent reindex happening in parallel would be able to wait for any activity and see any new rows inserted (or deleted). An isolation test is added to check after the case fixed here, which is a bit fancy by design as it relies on allow_system_table_mods to rename the toast table and its index to fixed names. This way, it is possible to reindex them directly without any dependency on the OID of the underlying relation. Note that this could not use a DO block either, as REINDEX CONCURRENTLY cannot be run in a transaction block. The test is backpatched down to 13, where it is possible, thanks to c4a7a39, to use allow_system_table_mods in a test suite. Reported-by: Alexey Ermakov Analyzed-by: Andres Freund, Noah Misch Author: Michael Paquier Reviewed-by: Nathan Bossart Discussion: https://postgr.es/m/17268-d2fb426e0895abd4@postgresql.org Backpatch-through: 12
1 parent 14c54e4 commit 9acea52

File tree

4 files changed

+905
-6
lines changed

4 files changed

+905
-6
lines changed

‎src/backend/access/common/toast_internals.c

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -344,10 +344,12 @@ toast_save_datum(Relation rel, Datum value,
344344
}
345345

346346
/*
347-
* Done - close toast relation and its indexes
347+
* Done - close toast relation and its indexes but keep the lock until
348+
* commit, so as a concurrent reindex done directly on the toast relation
349+
* would be able to wait for this transaction.
348350
*/
349-
toast_close_indexes(toastidxs, num_indexes, RowExclusiveLock);
350-
table_close(toastrel, RowExclusiveLock);
351+
toast_close_indexes(toastidxs, num_indexes, NoLock);
352+
table_close(toastrel, NoLock);
351353

352354
/*
353355
* Create the TOAST pointer value that we'll return
@@ -424,11 +426,13 @@ toast_delete_datum(Relation rel, Datum value, bool is_speculative)
424426
}
425427

426428
/*
427-
* End scan and close relations
429+
* End scan and close relations but keep the lock until commit, so as a
430+
* concurrent reindex done directly on the toast relation would be able to
431+
* wait for this transaction.
428432
*/
429433
systable_endscan_ordered(toastscan);
430-
toast_close_indexes(toastidxs, num_indexes, RowExclusiveLock);
431-
table_close(toastrel, RowExclusiveLock);
434+
toast_close_indexes(toastidxs, num_indexes, NoLock);
435+
table_close(toastrel, NoLock);
432436
}
433437

434438
/* ----------

0 commit comments

Comments
 (0)