Re: Add parallel columns for seq scan and index scan on pg_stat_all_tables and _indexes

Hi,

Le mer. 4 sept. 2024 à 16:18, Bertrand Drouvot <bertranddrouvot(dot)pg(at)gmail(dot)com>
a écrit :

> Hi,
>
> On Wed, Sep 04, 2024 at 02:51:51PM +0200, Guillaume Lelarge wrote:
> > Hi,
> >
> > Le mer. 4 sept. 2024 à 10:47, Bertrand Drouvot <
> bertranddrouvot(dot)pg(at)gmail(dot)com>
> > a écrit :
> >
> > > What about to get rid of the pgstat_count_parallel_heap_scan and add an
> > > extra
> > > bolean parameter to pgstat_count_heap_scan to indicate if
> > > counts.parallelnumscans
> > > should be incremented too?
> > >
> > > Something like:
> > >
> > > pgstat_count_heap_scan(scan->rs_base.rs_rd, scan->rs_base.rs_parallel
> !=
> > > NULL)
> > >
> > > 3 ===
> > >
> > > Same comment for pgstat_count_index_scan (add an extra bolean
> parameter)
> > > and
> > > get rid of pgstat_count_parallel_index_scan()).
> > >
> > > I think that 2 === and 3 === would help to avoid missing increments
> should
> > > we
> > > add those call to other places in the future.
> > >
> > >
> > Oh OK, understood. Done for both.
>
> Thanks for v2!
>
> 1 ===
>
> -#define pgstat_count_heap_scan(rel)
> +#define pgstat_count_heap_scan(rel, parallel)
> do {
> - if (pgstat_should_count_relation(rel))
> - (rel)->pgstat_info->counts.numscans++;
> + if (pgstat_should_count_relation(rel)) {
> + if (!parallel)
> + (rel)->pgstat_info->counts.numscans++;
> + else
> +
> (rel)->pgstat_info->counts.parallelnumscans++;
> + }
>
> I think counts.numscans has to be incremented in all the cases (so even if
> "parallel" is true).
>
> Same comment for pgstat_count_index_scan().
>
>
You're right, and I've been too quick. Fixed in v3.

> > 4 ===
> > >
> > > + if (lstats->counts.numscans || lstats->counts.parallelnumscans)
> > >
> > > Is it possible to have (lstats->counts.parallelnumscans) whithout
> having
> > > (lstats->counts.numscans) ?
> > >
> > >
> > Nope, parallel scans are included in seq/index scans, as far as I can
> tell.
> > I could remove the parallelnumscans testing but it would be less obvious
> to
> > read.
>
> 2 ===
>
> What about adding a comment instead of this extra check?
>
>
Done too in v3.

--
Guillaume.