From 5317f527f4a502347782030e30ca05db83c57584 Mon Sep 17 00:00:00 2001 From: amit Date: Fri, 6 Nov 2015 17:10:13 +0900 Subject: [PATCH 1/3] Make repack_drop() processing robust against deadlocks. Concurrent activity on the target table can cause deadlocks when repack_drop() is doing its job, ie, dropping the temporary objects created. It is highly likely to occur when pg_repack is interrupted midway through its processing. --- lib/repack.c | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/lib/repack.c b/lib/repack.c index 3224544..edac638 100644 --- a/lib/repack.c +++ b/lib/repack.c @@ -949,6 +949,17 @@ repack_drop(PG_FUNCTION_ARGS) /* connect to SPI manager */ repack_init(); + /* + * To prevent concurrent lockers of the repack target table from + * causing deadlocks, take an exclusive lock on it. + * + * Fixes deadlock mentioned in the Github issue #55. + */ + execute_with_format( + SPI_OK_UTILITY, + "LOCK TABLE %s IN ACCESS EXCLUSIVE MODE", + relname); + /* drop log table: must be done before dropping the pk type, * since the log table is dependent on the pk type. (That's * why we check numobj > 1 here.) From 94232991b90a64669ac1eeea36fb2d60cb924401 Mon Sep 17 00:00:00 2001 From: amit Date: Mon, 9 Nov 2015 15:23:45 +0900 Subject: [PATCH 2/3] Expand a comment in repack_drop(). Previous text did not sufficiently explain why taking a lock on the target table would be necessary. --- lib/repack.c | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/lib/repack.c b/lib/repack.c index edac638..51dde27 100644 --- a/lib/repack.c +++ b/lib/repack.c @@ -950,8 +950,19 @@ repack_drop(PG_FUNCTION_ARGS) repack_init(); /* - * To prevent concurrent lockers of the repack target table from - * causing deadlocks, take an exclusive lock on it. + * To prevent concurrent lockers of the repack target table from causing + * deadlocks, take an exclusive lock on it. Consider that the following + * commands take exclusive lock on tables log_xxx and the target table + * itself when deleting the z_repack_trigger on it, while concurrent + * updaters require row exclusive lock on the target table and in + * addition, on the log_xxx table, because of the trigger. + * + * Consider how a deadlock could occur - if the DROP TABLE repack.log_%u + * gets a lock on log_%u table before a concurrent updater could get it + * but after the updater has obtained a lock on the target table, the + * subsequent DROP TRIGGER ... ON target-table would report a deadlock as + * it finds itself waiting for a lock on target-table held by the updater, + * which in turn, is waiting for lock on log_%u table. * * Fixes deadlock mentioned in the Github issue #55. */ From cbe027289a838bcd5820c9d970de7b8d544b4467 Mon Sep 17 00:00:00 2001 From: amit Date: Thu, 3 Dec 2015 13:32:03 +0900 Subject: [PATCH 3/3] Consider schemaname too when doing LOCK TABLE in repack_drop(). This was an oversight in a previous commit to fix the deadlock reported in Github issue #55. --- lib/repack.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/repack.c b/lib/repack.c index 51dde27..ede7ebc 100644 --- a/lib/repack.c +++ b/lib/repack.c @@ -968,8 +968,8 @@ repack_drop(PG_FUNCTION_ARGS) */ execute_with_format( SPI_OK_UTILITY, - "LOCK TABLE %s IN ACCESS EXCLUSIVE MODE", - relname); + "LOCK TABLE %s.%s IN ACCESS EXCLUSIVE MODE", + nspname, relname); /* drop log table: must be done before dropping the pk type, * since the log table is dependent on the pk type. (That's