RE: Parallel Bitmap Heap Scan reports per-worker stats in EXPLAIN ANALYZE

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

In response to

Responses

Browse pgsql-hackers by date

  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