Summary Sort workers Stats in EXPLAIN ANALYZE

There is some problem with the last patch, I have removed the `ExplainOpenWorker` call to fix.

And also, I have added a test case in explain.sql​ according to the code change.
________________________________
From: Jian Guo <gjian@vmware.com>
Sent: Monday, March 21, 2022 11:36
To: pgsql-hackers@lists.postgresql.org <pgsql-hackers@lists.postgresql.org>
Cc: Zhenghua Lyu <zlyu@vmware.com>
Subject: Summary Sort workers Stats in EXPLAIN ANALYZE

In current EXPLAIN ANALYZE implementation, the Sort Node stats from each workers are not summarized: https://github.com/postgres/postgres/blob/d4ba8b51c76300f06cc23f4d8a41d9f7210c4866/src/backend/commands/explain.c#L2762&lt;https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fpostgres%2Fpostgres%2Fblob%2Fd4ba8b51c76300f06cc23f4d8a41d9f7210c4866%2Fsrc%2Fbackend%2Fcommands%2Fexplain.c%23L2762&amp;data=04%7C01%7Cgjian%40vmware.com%7C0f2c3df25e8a46bdd84f08da0aebff59%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637834305971955895%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&amp;sdata=RXY0uDuK7cFraHJqwU%2FQv%2BXhq3n%2F2cO6nv%2BoxHTbmCM%3D&amp;reserved=0&gt;

When the worker number is large, it will print out huge amount of node details in the plan. I have created this patch to summarize the tuplesort stats by AverageSpaceUsed / PeakSpaceUsed, make it behave just like in `show_incremental_sort_group_info()`: https://github.com/postgres/postgres/blob/d4ba8b51c76300f06cc23f4d8a41d9f7210c4866/src/backend/commands/explain.c#L2890&lt;https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fpostgres%2Fpostgres%2Fblob%2Fd4ba8b51c76300f06cc23f4d8a41d9f7210c4866%2Fsrc%2Fbackend%2Fcommands%2Fexplain.c%23L2890&amp;data=04%7C01%7Cgjian%40vmware.com%7C0f2c3df25e8a46bdd84f08da0aebff59%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637834305971955895%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&amp;sdata=NotrZFufFycTmlVy3SKioUSWGzLalSu9SWCOccMXGCI%3D&amp;reserved=0&gt;

For a simple demo, with this explain statement：

-- Test sort stats summary
set force_parallel_mode=on;
select explain_filter('explain (analyze, summary off, timing off, costs off, format json) select * from tenk1 order by unique1');

Before this patch, we got plan like this:

"Node Type": "Sort", +
"Parent Relationship": "Outer", +
"Parallel Aware": false, +
"Async Capable": false, +
"Actual Rows": 10000, +
"Actual Loops": 1, +
"Sort Key": ["unique1"], +
"Workers": [ +
{ +
"Worker Number": 0, +
"Sort Method": "external merge",+
"Sort Space Used": 2496, +
"Sort Space Type": "Disk" +
} +
], +

After this patch, the effected plan is this:

"Node Type": "Sort", +
"Parent Relationship": "Outer", +
"Parallel Aware": false, +
"Async Capable": false, +
"Actual Rows": N, +
"Actual Loops": N, +
"Sort Key": ["unique1"], +
"Workers planned": N, +
"Sort Method": "external merge", +
"Average Sort Space Used": N, +
"Peak Sort Space Used": N, +
"Sort Space Type": "Disk", +
________________________________
From: Jian Guo <gjian@vmware.com>
Sent: Monday, March 21, 2022 15:50
To: pgsql-hackers@lists.postgresql.org <pgsql-hackers@lists.postgresql.org>
Cc: Zhenghua Lyu <zlyu@vmware.com>
Subject: Re: Summary Sort workers Stats in EXPLAIN ANALYZE

There is some problem with the last patch, I have removed the `ExplainOpenWorker` call to fix.

And also, I have added a test case in explain.sql​ according to the code change.
________________________________
From: Jian Guo <gjian@vmware.com>
Sent: Monday, March 21, 2022 11:36
To: pgsql-hackers@lists.postgresql.org <pgsql-hackers@lists.postgresql.org>
Cc: Zhenghua Lyu <zlyu@vmware.com>
Subject: Summary Sort workers Stats in EXPLAIN ANALYZE

In current EXPLAIN ANALYZE implementation, the Sort Node stats from each workers are not summarized: https://github.com/postgres/postgres/blob/d4ba8b51c76300f06cc23f4d8a41d9f7210c4866/src/backend/commands/explain.c#L2762&lt;https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fpostgres%2Fpostgres%2Fblob%2Fd4ba8b51c76300f06cc23f4d8a41d9f7210c4866%2Fsrc%2Fbackend%2Fcommands%2Fexplain.c%23L2762&amp;data=04%7C01%7Cgjian%40vmware.com%7C0f2c3df25e8a46bdd84f08da0aebff59%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637834305971955895%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&amp;sdata=RXY0uDuK7cFraHJqwU%2FQv%2BXhq3n%2F2cO6nv%2BoxHTbmCM%3D&amp;reserved=0&gt;

When the worker number is large, it will print out huge amount of node details in the plan. I have created this patch to summarize the tuplesort stats by AverageSpaceUsed / PeakSpaceUsed, make it behave just like in `show_incremental_sort_group_info()`: https://github.com/postgres/postgres/blob/d4ba8b51c76300f06cc23f4d8a41d9f7210c4866/src/backend/commands/explain.c#L2890&lt;https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fpostgres%2Fpostgres%2Fblob%2Fd4ba8b51c76300f06cc23f4d8a41d9f7210c4866%2Fsrc%2Fbackend%2Fcommands%2Fexplain.c%23L2890&amp;data=04%7C01%7Cgjian%40vmware.com%7C0f2c3df25e8a46bdd84f08da0aebff59%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637834305971955895%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&amp;sdata=NotrZFufFycTmlVy3SKioUSWGzLalSu9SWCOccMXGCI%3D&amp;reserved=0&gt;

Hi,

On Thu, Mar 24, 2022 at 07:50:11AM +0000, Jian Guo wrote:

For a simple demo, with this explain statement：

-- Test sort stats summary
set force_parallel_mode=on;
select explain_filter('explain (analyze, summary off, timing off, costs off, format json) select * from tenk1 order by unique1');

Before this patch, we got plan like this:

"Node Type": "Sort", +
"Parent Relationship": "Outer", +
"Parallel Aware": false, +
"Async Capable": false, +
"Actual Rows": 10000, +
"Actual Loops": 1, +
"Sort Key": ["unique1"], +
"Workers": [ +
{ +
"Worker Number": 0, +
"Sort Method": "external merge",+
"Sort Space Used": 2496, +
"Sort Space Type": "Disk" +
} +
], +

After this patch, the effected plan is this:

"Node Type": "Sort", +
"Parent Relationship": "Outer", +
"Parallel Aware": false, +
"Async Capable": false, +
"Actual Rows": N, +
"Actual Loops": N, +
"Sort Key": ["unique1"], +
"Workers planned": N, +
"Sort Method": "external merge", +
"Average Sort Space Used": N, +
"Peak Sort Space Used": N, +
"Sort Space Type": "Disk", +

I think the idea is interesting, however there are a few problems in the patch.

First, I think that it should only be done in the VERBOSE OFF mode. If you ask
for a VERBOSE output you don't need both the details and the summarized
version.

Other minor problems:

- why (only) emitting the number of workers planned and not the number of
workers launched?
- the textual format is missing details about what the numbers are, which is
particularly obvious since avgSpaceUsed and peakSpaceUsed don't have any unit
or even space between them:

+			 "Sort Method: %s  %s: " INT64_FORMAT INT64_FORMAT "kB\n",
+			 sortMethod, spaceType, avgSpaceUsed, peakSpaceUsed);

On Fri, Mar 25, 2022 at 05:04:53PM +0800, Julien Rouhaud wrote:

I think the idea is interesting, however there are a few problems in the patch.

First, I think that it should only be done in the VERBOSE OFF mode. If you ask
for a VERBOSE output you don't need both the details and the summarized
version.

Other minor problems:

- why (only) emitting the number of workers planned and not the number of
workers launched?
- the textual format is missing details about what the numbers are, which is
particularly obvious since avgSpaceUsed and peakSpaceUsed don't have any unit
or even space between them:

+			 "Sort Method: %s  %s: " INT64_FORMAT INT64_FORMAT "kB\n",
+			 sortMethod, spaceType, avgSpaceUsed, peakSpaceUsed);

Also I didn't find your patch in the next commitfest [1]https://commitfest.postgresql.org/38/. Please register it
to make sure that it's not forgotten. Not that we're already at the end of the
last pg15 commitfest, so this should be material for pg16.

[1]: https://commitfest.postgresql.org/38/

I have updated the patch addressing the review comments, but I didn't moved this code block into VERBOSE mode, to keep consistency with `show_incremental_sort_info`:

https://github.com/postgres/postgres/blob/d4ba8b51c76300f06cc23f4d8a41d9f7210c4866/src/backend/commands/explain.c#L2890

Please review, thanks.

________________________________
From: Julien Rouhaud <rjuju123@gmail.com>
Sent: Friday, March 25, 2022 17:04
To: Jian Guo <gjian@vmware.com>
Cc: pgsql-hackers@lists.postgresql.org <pgsql-hackers@lists.postgresql.org>; Zhenghua Lyu <zlyu@vmware.com>
Subject: Re: Summary Sort workers Stats in EXPLAIN ANALYZE

⚠ External Email

Hi,

On Thu, Mar 24, 2022 at 07:50:11AM +0000, Jian Guo wrote:

For a simple demo, with this explain statement：

-- Test sort stats summary
set force_parallel_mode=on;
select explain_filter('explain (analyze, summary off, timing off, costs off, format json) select * from tenk1 order by unique1');

Before this patch, we got plan like this:

"Node Type": "Sort", +
"Parent Relationship": "Outer", +
"Parallel Aware": false, +
"Async Capable": false, +
"Actual Rows": 10000, +
"Actual Loops": 1, +
"Sort Key": ["unique1"], +
"Workers": [ +
{ +
"Worker Number": 0, +
"Sort Method": "external merge",+
"Sort Space Used": 2496, +
"Sort Space Type": "Disk" +
} +
], +

After this patch, the effected plan is this:

"Node Type": "Sort", +
"Parent Relationship": "Outer", +
"Parallel Aware": false, +
"Async Capable": false, +
"Actual Rows": N, +
"Actual Loops": N, +
"Sort Key": ["unique1"], +
"Workers planned": N, +
"Sort Method": "external merge", +
"Average Sort Space Used": N, +
"Peak Sort Space Used": N, +
"Sort Space Type": "Disk", +

I think the idea is interesting, however there are a few problems in the patch.

First, I think that it should only be done in the VERBOSE OFF mode. If you ask
for a VERBOSE output you don't need both the details and the summarized
version.

Other minor problems:

- why (only) emitting the number of workers planned and not the number of
workers launched?
- the textual format is missing details about what the numbers are, which is
particularly obvious since avgSpaceUsed and peakSpaceUsed don't have any unit
or even space between them:

+                        "Sort Method: %s  %s: " INT64_FORMAT INT64_FORMAT "kB\n",
+                        sortMethod, spaceType, avgSpaceUsed, peakSpaceUsed);

________________________________

⚠ External Email: This email originated from outside of the organization. Do not click links or open attachments unless you recognize the sender.

On Mon, Mar 28, 2022 at 2:55 PM Jian Guo <gjian@vmware.com> wrote:

I have updated the patch addressing the review comments, but I didn't
moved this code block into VERBOSE mode, to keep consistency with `
show_incremental_sort_info`:

https://github.com/postgres/postgres/blob/d4ba8b51c76300f06cc23f4d8a41d9f7210c4866/src/backend/commands/explain.c#L2890

Please review, thanks.

------------------------------
*From:* Julien Rouhaud <rjuju123@gmail.com>
*Sent:* Friday, March 25, 2022 17:04
*To:* Jian Guo <gjian@vmware.com>
*Cc:* pgsql-hackers@lists.postgresql.org <
pgsql-hackers@lists.postgresql.org>; Zhenghua Lyu <zlyu@vmware.com>
*Subject:* Re: Summary Sort workers Stats in EXPLAIN ANALYZE

⚠ External Email

Hi,

On Thu, Mar 24, 2022 at 07:50:11AM +0000, Jian Guo wrote:

For a simple demo, with this explain statement：

-- Test sort stats summary
set force_parallel_mode=on;
select explain_filter('explain (analyze, summary off, timing off, costs

off, format json) select * from tenk1 order by unique1');

Before this patch, we got plan like this:

"Node Type": "Sort", +
"Parent Relationship": "Outer", +
"Parallel Aware": false, +
"Async Capable": false, +
"Actual Rows": 10000, +
"Actual Loops": 1, +
"Sort Key": ["unique1"], +
"Workers": [ +
{ +
"Worker Number": 0, +
"Sort Method": "external merge",+
"Sort Space Used": 2496, +
"Sort Space Type": "Disk" +
} +
], +

After this patch, the effected plan is this:

"Node Type": "Sort", +
"Parent Relationship": "Outer", +
"Parallel Aware": false, +
"Async Capable": false, +
"Actual Rows": N, +
"Actual Loops": N, +
"Sort Key": ["unique1"], +
"Workers planned": N, +
"Sort Method": "external merge", +
"Average Sort Space Used": N, +
"Peak Sort Space Used": N, +
"Sort Space Type": "Disk", +

I think the idea is interesting, however there are a few problems in the
patch.

First, I think that it should only be done in the VERBOSE OFF mode. If
you ask
for a VERBOSE output you don't need both the details and the summarized
version.

Other minor problems:

- why (only) emitting the number of workers planned and not the number of
workers launched?
- the textual format is missing details about what the numbers are, which
is
particularly obvious since avgSpaceUsed and peakSpaceUsed don't have any
unit
or even space between them:

+                        "Sort Method: %s  %s: " INT64_FORMAT INT64_FORMAT
"kB\n",
+                        sortMethod, spaceType, avgSpaceUsed,
peakSpaceUsed);

________________________________

⚠ External Email: This email originated from outside of the organization.
Do not click links or open attachments unless you recognize the sender.

The patch failed different regression tests on all platforms. Please
correct that and send an updated patch.

[06:40:02.370] Test Summary Report
[06:40:02.370] -------------------
[06:40:02.370] t/002_pg_upgrade.pl (Wstat: 256 Tests: 13 Failed: 1)
[06:40:02.370] Failed test: 4
[06:40:02.370] Non-zero exit status: 1
[06:40:02.370] Files=2, Tests=21, 45 wallclock secs ( 0.02 usr 0.00 sys +
3.52 cusr 2.06 csys = 5.60 CPU)
--
Ibrar Ahmed

On Tue, Sep 06, 2022 at 11:37:32AM +0500, Ibrar Ahmed wrote:

The patch failed different regression tests on all platforms. Please
correct that and send an updated patch.

[06:40:02.370] Test Summary Report
[06:40:02.370] -------------------
[06:40:02.370] t/002_pg_upgrade.pl (Wstat: 256 Tests: 13 Failed: 1)
[06:40:02.370] Failed test: 4
[06:40:02.370] Non-zero exit status: 1
[06:40:02.370] Files=2, Tests=21, 45 wallclock secs ( 0.02 usr 0.00 sys +
3.52 cusr 2.06 csys = 5.60 CPU)

This has been marked as RwF based on the lack of an update.
--
Michael