mirrors/postgres
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Fix possible core dump in parallel restore when using a TOC list.

Browse Source 
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
This commit is contained in:
Tom Lane 2017-08-19 13:39:37 -04:00
parent 24620fc52b
commit b1c2d76a2f
1 changed files with 14 additions and 9 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

23
src/bin/pg_dump/pg_backup_archiver.c
View File

@ -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);