From 1c3869c0bea4376bcbb358e1abf5ed95e8c41865 Mon Sep 17 00:00:00 2001
From: Tom Lane <tgl@sss.pgh.pa.us>
Date: Sat, 19 Aug 2017 13:39:37 -0400
Subject: [PATCH] Fix possible core dump in parallel restore when using a TOC
list.
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Commit 3eb9a5e7c unintentionally introduced an ordering dependency
into restore_toc_entries_prefork(). The existing coding of
reduce_dependencies() contains a check to skip moving a TOC entry
to the ready_list if it wasn't initially in the pending_list.
This used to suffice to prevent reduce_dependencies() from trying to
move anything into the ready_list during restore_toc_entries_prefork(),
because the pending_list stayed empty throughout that phase; but it no
longer does. The problem doesn't manifest unless the TOC has been
reordered by SortTocFromFile, which is how I missed it in testing.
To fix, just add a test for ready_list == NULL, converting the call
with NULL from a poor man's sanity check into an explicit command
not to touch TOC items' list membership. Clarify some of the comments
around this; in particular, note the primary purpose of the check for
pending_list membership, which is to ensure that we can't try to restore
the same item twice, in case a TOC list forces it to be restored before
its dependency count goes to zero.
Per report from FabrÃzio de Royes Mello. Back-patch to 9.3, like the
previous commit.
Discussion: https://postgr.es/m/CAFcNs+pjuv0JL_x4+=71TPUPjdLHOXA4YfT32myj_OrrZb4ohA@mail.gmail.com
---
src/bin/pg_dump/pg_backup_archiver.c | 23 ++++++++++++++---------
1 file changed, 14 insertions(+), 9 deletions(-)
diff --git a/src/bin/pg_dump/pg_backup_archiver.c b/src/bin/pg_dump/pg_backup_archiver.c
index 4cfb71c0133..8ae7515c9cd 100644
--- a/src/bin/pg_dump/pg_backup_archiver.c
+++ b/src/bin/pg_dump/pg_backup_archiver.c
@@ -3851,8 +3851,9 @@ restore_toc_entries_prefork(ArchiveHandle *AH, TocEntry *pending_list)
*
* Note: as of 9.2, it should be guaranteed that all PRE_DATA items appear
* before DATA items, and all DATA items before POST_DATA items. That is
- * not certain to be true in older archives, though, so this loop is coded
- * to not assume it.
+ * not certain to be true in older archives, though, and in any case use
+ * of a list file would destroy that ordering (cf. SortTocFromFile). So
+ * this loop cannot assume that it holds.
*/
AH->restorePass = RESTORE_PASS_MAIN;
skipped_some = false;
@@ -3899,7 +3900,7 @@ restore_toc_entries_prefork(ArchiveHandle *AH, TocEntry *pending_list)
(void) restore_toc_entry(AH, next_work_item, false);
- /* there should be no touch of ready_list here, so pass NULL */
+ /* Reduce dependencies, but don't move anything to ready_list */
reduce_dependencies(AH, next_work_item, NULL);
}
else
@@ -4545,7 +4546,7 @@ identify_locking_dependencies(ArchiveHandle *AH, TocEntry *te)
/*
* Remove the specified TOC entry from the depCounts of items that depend on
* it, thereby possibly making them ready-to-run. Any pending item that
- * becomes ready should be moved to the ready list.
+ * becomes ready should be moved to the ready_list, if that's provided.
*/
static void
reduce_dependencies(ArchiveHandle *AH, TocEntry *te, TocEntry *ready_list)
@@ -4562,15 +4563,19 @@ reduce_dependencies(ArchiveHandle *AH, TocEntry *te, TocEntry *ready_list)
otherte->depCount--;
/*
- * It's ready if it has no remaining dependencies and it belongs in
- * the current restore pass. However, don't move it if it has not yet
- * been put into the pending list.
+ * It's ready if it has no remaining dependencies, and it belongs in
+ * the current restore pass, and it is currently a member of the
+ * pending list (that check is needed to prevent double restore in
+ * some cases where a list-file forces out-of-order restoring).
+ * However, if ready_list == NULL then caller doesn't want any list
+ * memberships changed.
*/
if (otherte->depCount == 0 &&
_tocEntryRestorePass(otherte) == AH->restorePass &&
- otherte->par_prev != NULL)
+ otherte->par_prev != NULL &&
+ ready_list != NULL)
{
- /* It must be in the pending list, so remove it ... */
+ /* Remove it from pending list ... */
par_list_remove(otherte);
/* ... and add to ready_list */
par_list_append(ready_list, otherte);
--
GitLab