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>, <Masao(dot)Fujii(at)nttdata(dot)com>
Subject: RE: Parallel Bitmap Heap Scan reports per-worker stats in EXPLAIN ANALYZE
Date: 2024-06-26 10:27:09
Message-ID: TYWPR01MB1098297D13C4841311F5344AFB1D62@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

Browse pgsql-hackers by date

  From Date Subject
Next Message Ashutosh Bapat 2024-06-26 10:37:31 pgindent exit status if a file encounters an error
Previous Message Bertrand Drouvot 2024-06-26 10:24:41 Re: Avoid orphaned objects dependencies, take 3