From: | <Masahiro(dot)Ikeda(at)nttdata(dot)com> |
---|---|
To: | <lena(dot)ribackina(at)yandex(dot)ru>, <donghanglin(at)gmail(dot)com> |
Cc: | <geidav(dot)pg(at)gmail(dot)com>, <melanieplageman(at)gmail(dot)com>, <tomas(dot)vondra(at)enterprisedb(dot)com>, <dilipbalaut(at)gmail(dot)com>, <pgsql-hackers(at)lists(dot)postgresql(dot)org>, <hlinnaka(at)iki(dot)fi> |
Subject: | RE: Parallel Bitmap Heap Scan reports per-worker stats in EXPLAIN ANALYZE |
Date: | 2024-06-26 10:22:23 |
Message-ID: | TYWPR01MB10982C38F6BD6BE4D1D5CCC5EB1D62@TYWPR01MB10982.jpnprd01.prod.outlook.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
Hi,
Thanks for working it! I'm interested in this feature, so I'd like to participate in the
patch review. Though I've just started looking at the patch, I have two comments
about the v6 patch. (And I want to confirm the thread active.)
1) Unify the print format of leader and worker
In show_tidbitmap_info(), the number of exact/loosy blocks of the leader and workers
are printed. I think the printed format should be same. Currently, the leader does not
print the blocks of exact/lossy with a value of 0, but the workers could even if it is 0.
IMHO, it's better to print both exact/lossy blocks if at least one of the numbers of
exact/lossy blocks is greater than 0. After all, the print logic is redundant for leader
and workers, but I thought it would be better to make it a common function.
2) Move es->workers_state check
In show_tidbitmap_info(), ExplainOpenWorker() and ExplainCloseWorker() are called
after checking es->worker_state is not NULL. However, es->workers_state seem to be
able to be NULL only for the Gather node (I see ExplainPrintPlan()). Also, reading the
comments, there is a description that each worker information needs to be hidden
when printing the plan.
Even if es->workers_state becomes NULL in BitmapHeapScan node in the future,
I think that workers' information(Heap Blocks) should not be printed. Therefore,
I think es->workers_state check should be move to the place of
"if (planstate->pstate != NULL)" like ExplainNode(), doesn't it?
IIUC, we need to correct show_sort_info() and so on too…
Regards,
--
Masahiro Ikeda
NTT DATA CORPORATION
From | Date | Subject | |
---|---|---|---|
Next Message | Bertrand Drouvot | 2024-06-26 10:24:41 | Re: Avoid orphaned objects dependencies, take 3 |
Previous Message | Jelte Fennema-Nio | 2024-06-26 09:36:27 | Re: RFC: Additional Directory for Extensions |