Re: Inefficiency in parallel pg_restore with many tables

From: Nathan Bossart <nathandbossart(at)gmail(dot)com>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Robert Haas <robertmhaas(at)gmail(dot)com>, Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org>, Andrew Dunstan <andrew(at)dunslane(dot)net>, pgsql-hackers(at)lists(dot)postgresql(dot)org
Subject: Re: Inefficiency in parallel pg_restore with many tables
Date: 2023-09-14 03:45:39
Message-ID: 20230914034539.GA1468515@nathanxps13
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Wed, Sep 13, 2023 at 08:01:39PM -0400, Tom Lane wrote:
> Nathan Bossart <nathandbossart(at)gmail(dot)com> writes:
>> Upon closer inspection, I found a rather nasty problem. The qsort
>> comparator expects a TocEntry **, but the binaryheap comparator expects a
>> TocEntry *, and we simply pass the arguments through to the qsort
>> comparator. In v9, I added the requisite ampersands.
>
> Ooops :-(
>
>> I'm surprised this
>> worked at all.
>
> Probably it was not sorting things appropriately. Might be worth adding
> some test scaffolding to check that bigger tasks are chosen before
> smaller ones.

Further testing revealed that the binaryheap comparator function was
actually generating a min-heap since the qsort comparator sorts by
decreasing dataLength. This is fixed in v10. And I am 0 for 2 today...

Now that this appears to be functioning as expected, I see that the larger
entries are typically picked up earlier, but we do sometimes pick entries
quite a bit further down the list, as anticipated. The case I was testing
(10k tables with the number of rows equal to the table number) was much
faster with this patch (just over a minute) than without it (over 16
minutes).

Sincerest apologies for the noise.

--
Nathan Bossart
Amazon Web Services: https://aws.amazon.com

Attachment Content-Type Size
v10-0001-Make-binaryheap-available-to-frontend-code.patch text/x-diff 8.8 KB
v10-0002-Add-function-for-removing-arbitrary-nodes-in-bin.patch text/x-diff 2.6 KB
v10-0003-Convert-pg_restore-s-ready_list-to-a-priority-qu.patch text/x-diff 15.8 KB
v10-0004-Remove-open-coded-binary-heap-in-pg_dump_sort.c.patch text/x-diff 6.1 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Dilip Kumar 2023-09-14 03:51:05 Re: [PoC] pg_upgrade: allow to upgrade publisher node
Previous Message Dilip Kumar 2023-09-14 03:44:40 Re: [PoC] pg_upgrade: allow to upgrade publisher node