On Wed, 26 Jun 2024 at 22:22, <Masahiro.Ikeda@nttdata.com> wrote:
> 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.
I agree with this. The two should match. I've fixed that in the attached.
I also made a pass over the patch, and I also changed:
1. Fixed up a few outdated comments in execnodes.h.
2. Added a comment in ExecEndBitmapHeapScan() to explain why we += the
stats rather than memcpy the BitmapHeapScanInstrumentation.
3. A bunch of other comments.
4. updated typedefs.list and ran pgindent.
For #2, I was surprised at this. I think there's probably a bug in the
Memoize stats code for the same reason. I've not looked into that yet.
I find it a little bit strange that we're showing stats for Worker N
when that worker could have been made up from possibly hundreds of
different parallel workers in the case where the Gather/GatherMerge
node is rescanned and the worker gets shut down at the end of each
Gather and fresh ones started up on rescan. I do agree that we need to
accumulate the totals from previous scans as that's what the
non-parallel version does.
Many people have been hacking on this and I'm wondering who should be
listed as authors. I plan to put David Geier first. Should anyone
else be listed there?
I've attached the rebased v5 patch with part of Alena's changes from
the diff.diff.no-cfbot file. I left the following one off as it looks
wrong.
- ptr += MAXALIGN(sizeof(ParallelBitmapHeapState));
+ ptr += size;
That would make ptr point to the end of the allocation.
I'd like to commit this patch soon, so if anyone wants to give it a
final look, can they do so before next week?
David
