Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit 41803d5

Browse files
committed
Fix possible core dump in parallel restore when using a TOC list.
Commit 3eb9a5e 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
1 parent c343314 commit 41803d5

File tree

1 file changed

+14
-9
lines changed

1 file changed

+14
-9
lines changed

src/bin/pg_dump/pg_backup_archiver.c

+14-9
Original file line numberDiff line numberDiff line change
@@ -3796,8 +3796,9 @@ restore_toc_entries_prefork(ArchiveHandle *AH, TocEntry *pending_list)
37963796
*
37973797
* Note: as of 9.2, it should be guaranteed that all PRE_DATA items appear
37983798
* before DATA items, and all DATA items before POST_DATA items. That is
3799-
* not certain to be true in older archives, though, so this loop is coded
3800-
* to not assume it.
3799+
* not certain to be true in older archives, though, and in any case use
3800+
* of a list file would destroy that ordering (cf. SortTocFromFile). So
3801+
* this loop cannot assume that it holds.
38013802
*/
38023803
AH->restorePass = RESTORE_PASS_MAIN;
38033804
skipped_some = false;
@@ -3844,7 +3845,7 @@ restore_toc_entries_prefork(ArchiveHandle *AH, TocEntry *pending_list)
38443845

38453846
(void) restore_toc_entry(AH, next_work_item, false);
38463847

3847-
/* there should be no touch of ready_list here, so pass NULL */
3848+
/* Reduce dependencies, but don't move anything to ready_list */
38483849
reduce_dependencies(AH, next_work_item, NULL);
38493850
}
38503851
else
@@ -4521,7 +4522,7 @@ identify_locking_dependencies(ArchiveHandle *AH, TocEntry *te)
45214522
/*
45224523
* Remove the specified TOC entry from the depCounts of items that depend on
45234524
* it, thereby possibly making them ready-to-run. Any pending item that
4524-
* becomes ready should be moved to the ready list.
4525+
* becomes ready should be moved to the ready_list, if that's provided.
45254526
*/
45264527
static void
45274528
reduce_dependencies(ArchiveHandle *AH, TocEntry *te, TocEntry *ready_list)
@@ -4538,15 +4539,19 @@ reduce_dependencies(ArchiveHandle *AH, TocEntry *te, TocEntry *ready_list)
45384539
otherte->depCount--;
45394540

45404541
/*
4541-
* It's ready if it has no remaining dependencies and it belongs in
4542-
* the current restore pass. However, don't move it if it has not yet
4543-
* been put into the pending list.
4542+
* It's ready if it has no remaining dependencies, and it belongs in
4543+
* the current restore pass, and it is currently a member of the
4544+
* pending list (that check is needed to prevent double restore in
4545+
* some cases where a list-file forces out-of-order restoring).
4546+
* However, if ready_list == NULL then caller doesn't want any list
4547+
* memberships changed.
45444548
*/
45454549
if (otherte->depCount == 0 &&
45464550
_tocEntryRestorePass(otherte) == AH->restorePass &&
4547-
otherte->par_prev != NULL)
4551+
otherte->par_prev != NULL &&
4552+
ready_list != NULL)
45484553
{
4549-
/* It must be in the pending list, so remove it ... */
4554+
/* Remove it from pending list ... */
45504555
par_list_remove(otherte);
45514556
/* ... and add to ready_list */
45524557
par_list_append(ready_list, otherte);

0 commit comments

Comments
 (0)