Re: explain analyze output with parallel workers - question about meaning of information for explain.depesz.com

From: Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>
To: Robert Haas <robertmhaas(at)gmail(dot)com>
Cc: Thomas Munro <thomas(dot)munro(at)enterprisedb(dot)com>, Hubert Lubaczewski <depesz(at)depesz(dot)com>, pgsql-hackers mailing list <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: explain analyze output with parallel workers - question about meaning of information for explain.depesz.com
Date: 2017-12-06 06:05:09
Message-ID: CAA4eK1J9U6Oy6fRXs3YNRdXfG6X5Qg3hP-wOuCKD2NMKA9okLw@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Wed, Dec 6, 2017 at 12:18 AM, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
> On Tue, Dec 5, 2017 at 2:49 AM, Thomas Munro
> <thomas(dot)munro(at)enterprisedb(dot)com> wrote:
>> As for how to aggregate the information, isn't it reasonable to show
>> data from the last loop on the basis that it's representative?
>> Summing wouldn't make too much sense, because you didn't use that much
>> memory all at once.
>
> Sorts can be rescanned even without parallel query, so I guess we
> should try to make the parallel case kinda like the non-parallel case.
> If I'm not wrong, that will just use the stats from the most recent
> execution (i.e. the last loop) -- see show_sort_info() in explain.c.
>

Right and seeing that I have prepared the patch (posted above [1])
which fixes it such that it will resemble the non-parallel case.
Ideally, it would have obviated the need for my previous patch which
got committed as 778e78ae. However, now that is committed, I could
think of below options:

1. I shall rebase it atop what is committed and actually, I have done
that in the attached patch. I have also prepared a regression test
case patch just to show the output with and without the patch.
2. For sort node, we can fix it by having some local_info same as
shared_info in sort node and copy the shared_info in that or we could
reinstate the pointer to the DSM in ExecSortReInitializeDSM() by
looking it up in the TOC as suggested by Thomas. If we go this way,
then we need a similar fix for hash node as well.

[1] - https://www.postgresql.org/message-id/CAA4eK1JBj4YCEQKeTua5%3DBMXy7zW7zNOvoXomzBP%3Dkb_aqMF7w%40mail.gmail.com

--
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com

Attachment Content-Type Size
fix_accum_instr_parallel_workers_v5.patch application/octet-stream 5.1 KB
test_sort_stats_v1.1.patch application/octet-stream 2.2 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Amit Kapila 2017-12-06 06:07:18 Re: explain analyze output with parallel workers - question about meaning of information for explain.depesz.com
Previous Message Fabien COELHO 2017-12-06 05:58:00 Re: [HACKERS] pow support for pgbench