Thread

1. Add parallel columns for seq scan and index scan on pg_stat_all_tables and _indexes

   Guillaume Lelarge <guillaume@lelarge.info> — 2024-08-29T14:04:05Z

   Hello,
   
   This patch was a bit discussed on [1], and with more details on [2]. It
   introduces four new columns in pg_stat_all_tables:
   
   * parallel_seq_scan
   * last_parallel_seq_scan
   * parallel_idx_scan
   * last_parallel_idx_scan
   
   and two new columns in pg_stat_all_indexes:
   
   * parallel_idx_scan
   * last_parallel_idx_scan
   
   As Benoit said yesterday, the intent is to help administrators evaluate the
   usage of parallel workers in their databases and help configuring
   parallelization usage.
   
   A test script (test.sql) is attached. You can execute it with "psql -Xef
   test.sql your_database" (your_database should not contain a t1 table as it
   will be dropped and recreated).
   
   Here is its result, a bit commented:
   
   DROP TABLE IF EXISTS t1;
   DROP TABLE
   CREATE TABLE t1 (id integer);
   CREATE TABLE
   INSERT INTO t1 SELECT generate_series(1, 10_000_000);
   INSERT 0 10000000
   VACUUM ANALYZE t1;
   VACUUM
   SELECT relname, seq_scan, last_seq_scan, parallel_seq_scan,
   last_parallel_seq_scan FROM pg_stat_user_tables WHERE relname='t1'
   -[ RECORD 1 ]----------+---
   relname                | t1
   seq_scan               | 0
   last_seq_scan          |
   parallel_seq_scan      | 0
   last_parallel_seq_scan |
   
   ==> no scan at all, the table has just been created
   
   SELECT * FROM t1 LIMIT 1;
    id
   ----
     1
   (1 row)
   
   SELECT pg_sleep(1);
   SELECT relname, seq_scan, last_seq_scan, parallel_seq_scan,
   last_parallel_seq_scan FROM pg_stat_user_tables WHERE relname='t1'
   -[ RECORD 1 ]----------+------------------------------
   relname                | t1
   seq_scan               | 1
   last_seq_scan          | 2024-08-29 15:43:17.377182+02
   parallel_seq_scan      | 0
   last_parallel_seq_scan |
   
   ==> one sequential scan, no parallelization
   
   SELECT count(*) FROM t1;
     count
   ----------
    10000000
   (1 row)
   
   SELECT pg_sleep(1);
   SELECT relname, seq_scan, last_seq_scan, parallel_seq_scan,
   last_parallel_seq_scan FROM pg_stat_user_tables WHERE relname='t1'
   -[ RECORD 1 ]----------+------------------------------
   relname                | t1
   seq_scan               | 4
   last_seq_scan          | 2024-08-29 15:43:18.504533+02
   parallel_seq_scan      | 3
   last_parallel_seq_scan | 2024-08-29 15:43:18.504533+02
   
   ==> one parallel sequential scan
   ==> I use the default configuration, so parallel_leader_participation = on,
   max_parallel_workers_per_gather = 2
   ==> meaning 3 parallel sequential scans (1 leader, two workers)
   ==> take note that seq_scan was also incremented... we didn't change the
   previous behaviour for this column
   
   CREATE INDEX ON t1(id);
   CREATE INDEX
   SELECT
   indexrelname,idx_scan,last_idx_scan,parallel_idx_scan,last_parallel_idx_scan,idx_tup_read,idx_tup_fetch
   FROM pg_stat_user_indexes WHERE relname='t1'
   -[ RECORD 1 ]----------+----------
   indexrelname           | t1_id_idx
   idx_scan               | 0
   last_idx_scan          |
   parallel_idx_scan      | 0
   last_parallel_idx_scan |
   idx_tup_read           | 0
   idx_tup_fetch          | 0
   
   ==> no scan at all, the index has just been created
   
   SELECT * FROM t1 WHERE id=150000;
      id
   --------
    150000
   (1 row)
   
   SELECT pg_sleep(1);
   SELECT
   indexrelname,idx_scan,last_idx_scan,parallel_idx_scan,last_parallel_idx_scan,idx_tup_read,idx_tup_fetch
   FROM pg_stat_user_indexes WHERE relname='t1'
   -[ RECORD 1 ]----------+------------------------------
   indexrelname           | t1_id_idx
   idx_scan               | 1
   last_idx_scan          | 2024-08-29 15:43:22.020853+02
   parallel_idx_scan      | 0
   last_parallel_idx_scan |
   idx_tup_read           | 1
   idx_tup_fetch          | 0
   
   ==> one index scan, no parallelization
   
   SELECT * FROM t1 WHERE id BETWEEN 100000 AND 400000;
   SELECT pg_sleep(1);
    pg_sleep
   ----------
   
   (1 row)
   
   SELECT
   indexrelname,idx_scan,last_idx_scan,parallel_idx_scan,last_parallel_idx_scan,idx_tup_read,idx_tup_fetch
   FROM pg_stat_user_indexes WHERE relname='t1'
   -[ RECORD 1 ]----------+------------------------------
   indexrelname           | t1_id_idx
   idx_scan               | 2
   last_idx_scan          | 2024-08-29 15:43:23.136665+02
   parallel_idx_scan      | 0
   last_parallel_idx_scan |
   idx_tup_read           | 300002
   idx_tup_fetch          | 0
   
   ==> another index scan, no parallelization
   
   SELECT count(*) FROM t1 WHERE id BETWEEN 100000 AND 400000;
    count
   --------
    300001
   (1 row)
   
   SELECT pg_sleep(1);
   SELECT
   indexrelname,idx_scan,last_idx_scan,parallel_idx_scan,last_parallel_idx_scan,idx_tup_read,idx_tup_fetch
   FROM pg_stat_user_indexes WHERE relname='t1'
   -[ RECORD 1 ]----------+-----------------------------
   indexrelname           | t1_id_idx
   idx_scan               | 5
   last_idx_scan          | 2024-08-29 15:43:24.16057+02
   parallel_idx_scan      | 3
   last_parallel_idx_scan | 2024-08-29 15:43:24.16057+02
   idx_tup_read           | 600003
   idx_tup_fetch          | 0
   
   ==> one parallel index scan
   ==> same thing, 3 parallel index scans (1 leader, two workers)
   ==> also, take note that idx_scan was also incremented... we didn't change
   the previous behaviour for this column
   
   First time I had to add new columns to a statistics catalog. I'm actually
   not sure that we were right to change pg_proc.dat manually. We'll probably
   have to fix this.
   
   Documentation is done, but maybe we should also add that seq_scan and
   idx_scan also include parallel scan.
   
   Yet to be done: tests. Once there's an agreement on this patch, we'll work
   on the tests.
   
   This has been a collective work with Benoit Lobréau, Jehan-Guillaume de
   Rorthais, and Franck Boudehen.
   
   Thanks.
   
   Regards.
   
   [1]
   https://www.postgresql.org/message-id/flat/b4220d15-2e21-0e98-921b-b9892543cc93%40dalibo.com
   [2]
   https://www.postgresql.org/message-id/flat/d657df20-c4bf-63f6-e74c-cb85a81d0383%40dalibo.com
   
   
   -- 
   Guillaume.
   

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

   Bertrand Drouvot <bertranddrouvot.pg@gmail.com> — 2024-09-04T08:47:11Z

   Hi,
   
   On Thu, Aug 29, 2024 at 04:04:05PM +0200, Guillaume Lelarge wrote:
   > Hello,
   > 
   > This patch was a bit discussed on [1], and with more details on [2]. It
   > introduces four new columns in pg_stat_all_tables:
   > 
   > * parallel_seq_scan
   > * last_parallel_seq_scan
   > * parallel_idx_scan
   > * last_parallel_idx_scan
   > 
   > and two new columns in pg_stat_all_indexes:
   > 
   > * parallel_idx_scan
   > * last_parallel_idx_scan
   > 
   > As Benoit said yesterday, the intent is to help administrators evaluate the
   > usage of parallel workers in their databases and help configuring
   > parallelization usage.
   
   Thanks for the patch. I think that's a good idea to provide more instrumentation
   in this area. So, +1 regarding this patch.
   
   A few random comments:
   
   1 ===
   
   +     <row>
   +      <entry role="catalog_table_entry"><para role="column_definition">
   +       <structfield>parallel_seq_scan</structfield> <type>bigint</type>
   +      </para>
   +      <para>
   +       Number of parallel sequential scans initiated on this table
   +      </para></entry>
   +     </row>
   
   I wonder if we should not update the seq_scan too to indicate that it
   includes the parallel_seq_scan.
   
   Same kind of comment for last_seq_scan, idx_scan and last_idx_scan.
   
   2 ===
   
   @@ -410,6 +410,8 @@ initscan(HeapScanDesc scan, ScanKey key, bool keep_startblock)
            */
           if (scan->rs_base.rs_flags & SO_TYPE_SEQSCAN)
                   pgstat_count_heap_scan(scan->rs_base.rs_rd);
   +  if (scan->rs_base.rs_parallel != NULL)
   +               pgstat_count_parallel_heap_scan(scan->rs_base.rs_rd);
   
   Indentation seems broken.
   
   Shouldn't the parallel counter relies on the "scan->rs_base.rs_flags & SO_TYPE_SEQSCAN"
   test too?
   
   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.
   
   4 ===
   
   +       if (lstats->counts.numscans || lstats->counts.parallelnumscans)
   
   Is it possible to have (lstats->counts.parallelnumscans) whithout having
   (lstats->counts.numscans) ?
   
   > First time I had to add new columns to a statistics catalog. I'm actually
   > not sure that we were right to change pg_proc.dat manually.
   
   I think that's the right way to do.
   
   I don't see a CF entry for this patch. Would you mind creating one so that
   we don't lost track of it?
   
   Regards,
   
   -- 
   Bertrand Drouvot
   PostgreSQL Contributors Team
   RDS Open Source Databases
   Amazon Web Services: https://aws.amazon.com
   
   
   
   

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

   Guillaume Lelarge <guillaume@lelarge.info> — 2024-09-04T12:51:51Z

   Hi,
   
   Le mer. 4 sept. 2024 à 10:47, Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
   a écrit :
   
   > Hi,
   >
   > On Thu, Aug 29, 2024 at 04:04:05PM +0200, Guillaume Lelarge wrote:
   > > Hello,
   > >
   > > This patch was a bit discussed on [1], and with more details on [2]. It
   > > introduces four new columns in pg_stat_all_tables:
   > >
   > > * parallel_seq_scan
   > > * last_parallel_seq_scan
   > > * parallel_idx_scan
   > > * last_parallel_idx_scan
   > >
   > > and two new columns in pg_stat_all_indexes:
   > >
   > > * parallel_idx_scan
   > > * last_parallel_idx_scan
   > >
   > > As Benoit said yesterday, the intent is to help administrators evaluate
   > the
   > > usage of parallel workers in their databases and help configuring
   > > parallelization usage.
   >
   > Thanks for the patch. I think that's a good idea to provide more
   > instrumentation
   > in this area. So, +1 regarding this patch.
   >
   >
   Thanks.
   
   
   > A few random comments:
   >
   > 1 ===
   >
   > +     <row>
   > +      <entry role="catalog_table_entry"><para role="column_definition">
   > +       <structfield>parallel_seq_scan</structfield> <type>bigint</type>
   > +      </para>
   > +      <para>
   > +       Number of parallel sequential scans initiated on this table
   > +      </para></entry>
   > +     </row>
   >
   > I wonder if we should not update the seq_scan too to indicate that it
   > includes the parallel_seq_scan.
   >
   > Same kind of comment for last_seq_scan, idx_scan and last_idx_scan.
   >
   >
   Yeah, not sure why I didn't do it at first. I was wondering the same thing.
   The patch attached does this.
   
   
   > 2 ===
   >
   > @@ -410,6 +410,8 @@ initscan(HeapScanDesc scan, ScanKey key, bool
   > keep_startblock)
   >          */
   >         if (scan->rs_base.rs_flags & SO_TYPE_SEQSCAN)
   >                 pgstat_count_heap_scan(scan->rs_base.rs_rd);
   > +  if (scan->rs_base.rs_parallel != NULL)
   > +               pgstat_count_parallel_heap_scan(scan->rs_base.rs_rd);
   >
   > Indentation seems broken.
   >
   >
   My bad, sorry. Fixed in the attached patch.
   
   
   > Shouldn't the parallel counter relies on the "scan->rs_base.rs_flags &
   > SO_TYPE_SEQSCAN"
   > test too?
   >
   >
   You're right. Fixed in the attached patch.
   
   
   > 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.
   
   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.
   
   
   > > First time I had to add new columns to a statistics catalog. I'm actually
   > > not sure that we were right to change pg_proc.dat manually.
   >
   > I think that's the right way to do.
   >
   >
   OK, new patch attached.
   
   
   > I don't see a CF entry for this patch. Would you mind creating one so that
   > we don't lost track of it?
   >
   >
   I don't mind adding it, though I don't know if I should add it to the
   September or November commit fest. Which one should I choose?
   
   Thanks.
   
   Regards.
   
   
   -- 
   Guillaume.
   

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

   Bertrand Drouvot <bertranddrouvot.pg@gmail.com> — 2024-09-04T12:58:47Z

   Hi,
   
   On Wed, Sep 04, 2024 at 02:51:51PM +0200, Guillaume Lelarge wrote:
   > Le mer. 4 sept. 2024 à 10:47, Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
   > a écrit :
   > 
   > > I don't see a CF entry for this patch. Would you mind creating one so that
   > > we don't lost track of it?
   > >
   > >
   > I don't mind adding it, though I don't know if I should add it to the
   > September or November commit fest. Which one should I choose?
   
   Thanks! That should be the November one (as the September one already started).
   
   Regards,
   
   -- 
   Bertrand Drouvot
   PostgreSQL Contributors Team
   RDS Open Source Databases
   Amazon Web Services: https://aws.amazon.com
   
   
   
   

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

   Guillaume Lelarge <guillaume@lelarge.info> — 2024-09-04T13:25:03Z

   Le mer. 4 sept. 2024 à 14:58, Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
   a écrit :
   
   > Hi,
   >
   > On Wed, Sep 04, 2024 at 02:51:51PM +0200, Guillaume Lelarge wrote:
   > > Le mer. 4 sept. 2024 ą 10:47, Bertrand Drouvot <
   > bertranddrouvot.pg@gmail.com>
   > > a écrit :
   > >
   > > > I don't see a CF entry for this patch. Would you mind creating one so
   > that
   > > > we don't lost track of it?
   > > >
   > > >
   > > I don't mind adding it, though I don't know if I should add it to the
   > > September or November commit fest. Which one should I choose?
   >
   > Thanks! That should be the November one (as the September one already
   > started).
   >
   >
   I should have gone to the commit fest website, it says the same. I had the
   recollection that it started on the 15th. Anyway, added to the november
   commit fest (https://commitfest.postgresql.org/50/5238/).
   
   
   -- 
   Guillaume.
   

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

   Bertrand Drouvot <bertranddrouvot.pg@gmail.com> — 2024-09-04T14:18:57Z

   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.pg@gmail.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().
   
   > 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?
   
   Regards,
   
   -- 
   Bertrand Drouvot
   PostgreSQL Contributors Team
   RDS Open Source Databases
   Amazon Web Services: https://aws.amazon.com
   
   
   
   

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

   Guillaume Lelarge <guillaume@lelarge.info> — 2024-09-04T14:37:19Z

   Hi,
   
   Le mer. 4 sept. 2024 à 16:18, Bertrand Drouvot <bertranddrouvot.pg@gmail.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.pg@gmail.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.
   

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

   Bertrand Drouvot <bertranddrouvot.pg@gmail.com> — 2024-09-05T05:36:09Z

   Hi,
   
   On Wed, Sep 04, 2024 at 04:37:19PM +0200, Guillaume Lelarge wrote:
   > Hi,
   > 
   > Le mer. 4 sept. 2024 à 16:18, Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
   > a écrit :
   > > What about adding a comment instead of this extra check?
   > >
   > >
   > Done too in v3.
   
   Thanks!
   
   1 ===
   
   +       /*
   +        * Don't check counts.parallelnumscans because counts.numscans includes
   +        * counts.parallelnumscans
   +        */
   
   "." is missing at the end of the comment.
   
   2 ===
   
   -               if (t > tabentry->lastscan)
   +               if (t > tabentry->lastscan && lstats->counts.numscans)
   
   The extra check on lstats->counts.numscans is not needed as it's already done
   a few lines before.
   
   3 ===
   
   +               if (t > tabentry->parallellastscan && lstats->counts.parallelnumscans)
   
   This one makes sense.
    
   And now I'm wondering if the extra comment added in v3 is really worth it (and
   does not sound confusing)? I mean, the parallel check is done once we passe
   the initial test on counts.numscans. I think the code is clear enough without
   this extra comment, thoughts? 
   
   4 ===
   
   What about adding a few tests? or do you want to wait a bit more to see if "
   there's an agreement on this patch" (as you stated at the start of this thread).
   
   Regards,
   
   -- 
   Bertrand Drouvot
   PostgreSQL Contributors Team
   RDS Open Source Databases
   Amazon Web Services: https://aws.amazon.com
   
   
   
   

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

   Guillaume Lelarge <guillaume@lelarge.info> — 2024-09-05T06:19:13Z

   Le jeu. 5 sept. 2024 à 07:36, Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
   a écrit :
   
   > Hi,
   >
   > On Wed, Sep 04, 2024 at 04:37:19PM +0200, Guillaume Lelarge wrote:
   > > Hi,
   > >
   > > Le mer. 4 sept. 2024 à 16:18, Bertrand Drouvot <
   > bertranddrouvot.pg@gmail.com>
   > > a écrit :
   > > > What about adding a comment instead of this extra check?
   > > >
   > > >
   > > Done too in v3.
   >
   > Thanks!
   >
   > 1 ===
   >
   > +       /*
   > +        * Don't check counts.parallelnumscans because counts.numscans
   > includes
   > +        * counts.parallelnumscans
   > +        */
   >
   > "." is missing at the end of the comment.
   >
   >
   Fixed in v4.
   
   
   > 2 ===
   >
   > -               if (t > tabentry->lastscan)
   > +               if (t > tabentry->lastscan && lstats->counts.numscans)
   >
   > The extra check on lstats->counts.numscans is not needed as it's already
   > done
   > a few lines before.
   >
   >
   Fixed in v4.
   
   
   > 3 ===
   >
   > +               if (t > tabentry->parallellastscan &&
   > lstats->counts.parallelnumscans)
   >
   > This one makes sense.
   >
   > And now I'm wondering if the extra comment added in v3 is really worth it
   > (and
   > does not sound confusing)? I mean, the parallel check is done once we passe
   > the initial test on counts.numscans. I think the code is clear enough
   > without
   > this extra comment, thoughts?
   >
   >
   I'm not sure I understand you here. I kinda like the extra comment though.
   
   
   > 4 ===
   >
   > What about adding a few tests? or do you want to wait a bit more to see if
   > "
   > there's an agreement on this patch" (as you stated at the start of this
   > thread).
   >
   >
   Guess I can start working on that now. It will take some time as I've never
   done it before. Good thing I added the patch on the November commit fest :)
   
   Thanks again.
   
   Regards.
   
   
   -- 
   Guillaume.
   

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

    Guillaume Lelarge <guillaume@lelarge.info> — 2024-10-06T19:53:32Z

    Hello,
    
    Le jeu. 5 sept. 2024 à 08:19, Guillaume Lelarge <guillaume@lelarge.info> a
    écrit :
    
    > Le jeu. 5 sept. 2024 à 07:36, Bertrand Drouvot <
    > bertranddrouvot.pg@gmail.com> a écrit :
    >
    >> Hi,
    >>
    >> On Wed, Sep 04, 2024 at 04:37:19PM +0200, Guillaume Lelarge wrote:
    >> > Hi,
    >> >
    >> > Le mer. 4 sept. 2024 à 16:18, Bertrand Drouvot <
    >> bertranddrouvot.pg@gmail.com>
    >> > a écrit :
    >> > > What about adding a comment instead of this extra check?
    >> > >
    >> > >
    >> > Done too in v3.
    >>
    >> Thanks!
    >>
    >> 1 ===
    >>
    >> +       /*
    >> +        * Don't check counts.parallelnumscans because counts.numscans
    >> includes
    >> +        * counts.parallelnumscans
    >> +        */
    >>
    >> "." is missing at the end of the comment.
    >>
    >>
    > Fixed in v4.
    >
    >
    >> 2 ===
    >>
    >> -               if (t > tabentry->lastscan)
    >> +               if (t > tabentry->lastscan && lstats->counts.numscans)
    >>
    >> The extra check on lstats->counts.numscans is not needed as it's already
    >> done
    >> a few lines before.
    >>
    >>
    > Fixed in v4.
    >
    >
    >> 3 ===
    >>
    >> +               if (t > tabentry->parallellastscan &&
    >> lstats->counts.parallelnumscans)
    >>
    >> This one makes sense.
    >>
    >> And now I'm wondering if the extra comment added in v3 is really worth it
    >> (and
    >> does not sound confusing)? I mean, the parallel check is done once we
    >> passe
    >> the initial test on counts.numscans. I think the code is clear enough
    >> without
    >> this extra comment, thoughts?
    >>
    >>
    > I'm not sure I understand you here. I kinda like the extra comment though.
    >
    >
    >> 4 ===
    >>
    >> What about adding a few tests? or do you want to wait a bit more to see
    >> if "
    >> there's an agreement on this patch" (as you stated at the start of this
    >> thread).
    >>
    >>
    > Guess I can start working on that now. It will take some time as I've
    > never done it before. Good thing I added the patch on the November commit
    > fest :)
    >
    >
    
    Finally found some time to work on this. Tests added on v5 patch (attached).
    
    Regards.
    
    
    -- 
    Guillaume.
    

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

    Alena Rybakina <a.rybakina@postgrespro.ru> — 2024-10-06T21:43:18Z

    Hi!
    
    > Finally found some time to work on this. Tests added on v5 patch 
    > (attached).
    Maybe I'm not aware of the whole context of the thread and maybe my 
    questions will seem a bit stupid, but honestly
    it's not entirely clear to me how this statistics will help to adjust 
    the number of parallel workers.
    We may have situations when during overestimation of the cardinality 
    during query optimization a several number of parallel workers were 
    unjustifiably generated and vice versa -
    due to a high workload only a few number of workers were generated.
    How do we identify such cases so as not to increase or decrease the 
    number of parallel workers when it is not necessary?
    
    -- 
    Regards,
    Alena Rybakina
    Postgres Professional
    

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

    Michael Paquier <michael@paquier.xyz> — 2024-10-07T00:41:36Z

    On Mon, Oct 07, 2024 at 12:43:18AM +0300, Alena Rybakina wrote:
    > Maybe I'm not aware of the whole context of the thread and maybe my
    > questions will seem a bit stupid, but honestly
    > it's not entirely clear to me how this statistics will help to adjust the
    > number of parallel workers.
    > We may have situations when during overestimation of the cardinality during
    > query optimization a several number of parallel workers were unjustifiably
    > generated and vice versa -
    > due to a high workload only a few number of workers were generated.
    > How do we identify such cases so as not to increase or decrease the number
    > of parallel workers when it is not necessary?
    
    Well.  For spiky workloads, only these numbers are not going to help.
    If you can map them with the number of times a query related to these
    tables has been called, something that pg_stat_statements would be
    able to show more information about.
    
    FWIW, I have doubts that these numbers attached to this portion of the
    system are always useful.  For OLTP workloads, parallel workers would
    unlikely be spawned because even with JOINs we won't work with a high
    number of tuples that require them.  This could be interested with
    analytics, however complex query sequences mean that we'd still need
    to look at all the plans involving the relations where there is an
    unbalance of planned/spawned workers, because these can usually
    involve quite a few gather nodes.  At the end of the day, it seems to
    me that we would still need data that involves statements to track
    down specific plans that are starving.  If your application does not
    have that many statements, looking at individial plans is OK, but if
    you have hundreds of them to dig into, this is time-consuming and
    stats at table/index level don't offer data in terms of stuff that
    stands out and needs adjustments.
    
    And this is without the argument of bloating more the stats entries
    for each table, even if it matters less now that these stats are in
    shmem lately.
    --
    Michael
    

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

    Guillaume Lelarge <guillaume@lelarge.info> — 2024-10-07T06:34:58Z

    Le lun. 7 oct. 2024 à 02:41, Michael Paquier <michael@paquier.xyz> a écrit :
    
    > On Mon, Oct 07, 2024 at 12:43:18AM +0300, Alena Rybakina wrote:
    > > Maybe I'm not aware of the whole context of the thread and maybe my
    > > questions will seem a bit stupid, but honestly
    > > it's not entirely clear to me how this statistics will help to adjust the
    > > number of parallel workers.
    > > We may have situations when during overestimation of the cardinality
    > during
    > > query optimization a several number of parallel workers were
    > unjustifiably
    > > generated and vice versa -
    > > due to a high workload only a few number of workers were generated.
    > > How do we identify such cases so as not to increase or decrease the
    > number
    > > of parallel workers when it is not necessary?
    >
    > Well.  For spiky workloads, only these numbers are not going to help.
    > If you can map them with the number of times a query related to these
    > tables has been called, something that pg_stat_statements would be
    > able to show more information about.
    >
    > FWIW, I have doubts that these numbers attached to this portion of the
    > system are always useful.  For OLTP workloads, parallel workers would
    > unlikely be spawned because even with JOINs we won't work with a high
    > number of tuples that require them.  This could be interested with
    > analytics, however complex query sequences mean that we'd still need
    > to look at all the plans involving the relations where there is an
    > unbalance of planned/spawned workers, because these can usually
    > involve quite a few gather nodes.  At the end of the day, it seems to
    > me that we would still need data that involves statements to track
    > down specific plans that are starving.  If your application does not
    > have that many statements, looking at individial plans is OK, but if
    > you have hundreds of them to dig into, this is time-consuming and
    > stats at table/index level don't offer data in terms of stuff that
    > stands out and needs adjustments.
    >
    > And this is without the argument of bloating more the stats entries
    > for each table, even if it matters less now that these stats are in
    > shmem lately.
    >
    
    We need granularity because we have granularity in the config. There is
    pg_stat_database because it gives the whole picture and it is easier to
    monitor. And then, there is pg_stat_statements to analyze problematic
    statements. And finally there is pg_stat_all* because you can set
    parallel_workers on a specific table.
    
    Anyway, offering various ways of getting the same information is not
    unheard of. Pretty much like temp_files/temp_bytes in pg_stat_database,
    temp_blks_read/temp_blks_written in pg_stat_statements and log_temp_files
    in log files if you ask me :)
    
    
    -- 
    Guillaume.
    

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

    Alena Rybakina <a.rybakina@postgrespro.ru> — 2024-10-08T15:20:17Z

    On 07.10.2024 03:41, Michael Paquier wrote:
    > On Mon, Oct 07, 2024 at 12:43:18AM +0300, Alena Rybakina wrote:
    >> Maybe I'm not aware of the whole context of the thread and maybe my
    >> questions will seem a bit stupid, but honestly
    >> it's not entirely clear to me how this statistics will help to adjust the
    >> number of parallel workers.
    >> We may have situations when during overestimation of the cardinality during
    >> query optimization a several number of parallel workers were unjustifiably
    >> generated and vice versa -
    >> due to a high workload only a few number of workers were generated.
    >> How do we identify such cases so as not to increase or decrease the number
    >> of parallel workers when it is not necessary?
    > Well.  For spiky workloads, only these numbers are not going to help.
    > If you can map them with the number of times a query related to these
    > tables has been called, something that pg_stat_statements would be
    > able to show more information about.
    >
    > FWIW, I have doubts that these numbers attached to this portion of the
    > system are always useful.  For OLTP workloads, parallel workers would
    > unlikely be spawned because even with JOINs we won't work with a high
    > number of tuples that require them.  This could be interested with
    > analytics, however complex query sequences mean that we'd still need
    > to look at all the plans involving the relations where there is an
    > unbalance of planned/spawned workers, because these can usually
    > involve quite a few gather nodes.  At the end of the day, it seems to
    > me that we would still need data that involves statements to track
    > down specific plans that are starving.  If your application does not
    > have that many statements, looking at individial plans is OK, but if
    > you have hundreds of them to dig into, this is time-consuming and
    > stats at table/index level don't offer data in terms of stuff that
    > stands out and needs adjustments.
    >
    > And this is without the argument of bloating more the stats entries
    > for each table, even if it matters less now that these stats are in
    > shmem lately.
    
    To be honest, it’s not entirely clear to me how these statistics will 
    help in setting up parallel workers.
    
    As I understand, we need additional tools for analytics, which are 
    available in pg_stat_statements, but how then does it work? maybe you 
    have the opportunity to demonstrate this?
    
    -- 
    Regards,
    Alena Rybakina
    Postgres Professional
    
    
    
    
    

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

    Alena Rybakina <a.rybakina@postgrespro.ru> — 2024-10-08T15:24:54Z

    On 07.10.2024 09:34, Guillaume Lelarge wrote:
    > We need granularity because we have granularity in the config. There 
    > is pg_stat_database because it gives the whole picture and it is 
    > easier to monitor. And then, there is pg_stat_statements to analyze 
    > problematic statements. And finally there is pg_stat_all* because you 
    > can set parallel_workers on a specific table.
    >
    yes, I agree with you. Even when I experimented with vacuum settings for 
    database and used my vacuum statistics patch [0] for analyzes , I first 
    looked at this change in the number of blocks or deleted rows at the 
    database level,
    and only then did an analysis of each table and index.
    
    [0] https://commitfest.postgresql.org/50/5012/
    
    -- 
    Regards,
    Alena Rybakina
    Postgres Professional
    
    
    
    
    

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

    Michael Paquier <michael@paquier.xyz> — 2024-11-11T02:04:57Z

    On Tue, Oct 08, 2024 at 06:24:54PM +0300, Alena Rybakina wrote:
    > yes, I agree with you. Even when I experimented with vacuum settings for
    > database and used my vacuum statistics patch [0] for analyzes , I first
    > looked at this change in the number of blocks or deleted rows at the
    > database level,
    > and only then did an analysis of each table and index.
    > 
    > [0] https://commitfest.postgresql.org/50/5012/
    
    As hinted on other related threads like around [1], I am so-so about
    the proposal of these numbers at table and index level now that we
    have e7a9496de906 and 5d4298e75f25.
    
    In such cases, I apply the concept that I call the "Mention Bien" (or
    when you get a baccalaureat diploma with honors and with a 14~16/20 in
    France).  What we have is not perfect, still it's good enough to get
    a 14/20 IMO, making hopefully 70~80% of users happy with these new
    metrics.  Perhaps I'm wrong, but I'd be curious to know if this
    thread's proposal is required at all at the end.
    
    I have not looked at the logging proposal yet.
    
    [1]: https://www.postgresql.org/message-id/Zywxw7vqPLBfVfXN@paquier.xyz
    --
    Michael
    

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

    Guillaume Lelarge <guillaume@lelarge.info> — 2024-11-11T08:34:57Z

    Le lun. 11 nov. 2024 à 03:05, Michael Paquier <michael@paquier.xyz> a
    écrit :
    
    > On Tue, Oct 08, 2024 at 06:24:54PM +0300, Alena Rybakina wrote:
    > > yes, I agree with you. Even when I experimented with vacuum settings for
    > > database and used my vacuum statistics patch [0] for analyzes , I first
    > > looked at this change in the number of blocks or deleted rows at the
    > > database level,
    > > and only then did an analysis of each table and index.
    > >
    > > [0] https://commitfest.postgresql.org/50/5012/
    >
    > As hinted on other related threads like around [1], I am so-so about
    > the proposal of these numbers at table and index level now that we
    > have e7a9496de906 and 5d4298e75f25.
    >
    > In such cases, I apply the concept that I call the "Mention Bien" (or
    > when you get a baccalaureat diploma with honors and with a 14~16/20 in
    > France).  What we have is not perfect, still it's good enough to get
    > a 14/20 IMO, making hopefully 70~80% of users happy with these new
    > metrics.  Perhaps I'm wrong, but I'd be curious to know if this
    > thread's proposal is required at all at the end.
    >
    >
    I agree with you. We'll see if we need more, but it's already good to have
    the metrics already commited.
    
    
    > I have not looked at the logging proposal yet.
    >
    >
    I hope you'll have time to look at it. It seems to me very important to get
    that kind of info in the logs.
    
    Thanks again.
    
    
    > [1]: https://www.postgresql.org/message-id/Zywxw7vqPLBfVfXN@paquier.xyz
    > --
    > Michael
    >
    
    
    -- 
    Guillaume.
    

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

    Robert Haas <robertmhaas@gmail.com> — 2024-11-11T16:06:43Z

    On Sun, Nov 10, 2024 at 9:05 PM Michael Paquier <michael@paquier.xyz> wrote:
    > As hinted on other related threads like around [1], I am so-so about
    > the proposal of these numbers at table and index level now that we
    > have e7a9496de906 and 5d4298e75f25.
    
    I think the question to which we don't have a clear answer is: for
    what purpose would you want to be able to distinguish parallel and
    non-parallel scans on a per-table basis?
    
    I think it's fairly clear why the existing counters exist at a table
    level. If an index isn't getting very many index scans, perhaps it's
    useless -- or worse than useless if it interferes with HOT -- and
    should be dropped. On the other hand if a table is getting a lot of
    sequential scans even though it happens to be quite large, perhaps new
    indexes are needed. Or if the indexes that we expect to get used are
    not the same as those actually getting used, perhaps we want to add or
    drop indexes or adjust the queries.
    
    But it is unclear to me what sort of tuning we would do based on
    knowing how many of the scans on a certain table or a certain index
    were parallel vs non-parallel. I have not fully reviewed the threads
    linked in the original post; but I did look at them briefly and did
    not immediately see discussion of the specific counters proposed here.
    I also don't see anything in this thread that clearly explains why we
    should want this exact thing. I don't want to make it sound like I
    know that this is useless; I'm sure that Guillaume probably has lots
    of hands-on tuning experience with this stuff that I lack. But the
    reasons aren't clearly spelled out as far as I can see, and I'm having
    some trouble imagining what they are.
    
    Compare the parallel worker draught stuff. It's really clear how that
    is intended to be used. If we're routinely failing to launch workers,
    then either max_parallel_workers_per_gather is too high or
    max_parallel_workers is too low. Now, I will admit that I have a few
    doubts about whether that feature will get much real-world use but it
    seems hard to doubt that it HAS a use. In this case, that seems less
    clear.
    
    -- 
    Robert Haas
    EDB: http://www.enterprisedb.com
    
    
    
    

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

    Michael Paquier <michael@paquier.xyz> — 2024-11-12T03:41:19Z

    On Mon, Nov 11, 2024 at 11:06:43AM -0500, Robert Haas wrote:
    > But it is unclear to me what sort of tuning we would do based on
    > knowing how many of the scans on a certain table or a certain index
    > were parallel vs non-parallel. I have not fully reviewed the threads
    > linked in the original post; but I did look at them briefly and did
    > not immediately see discussion of the specific counters proposed here.
    > I also don't see anything in this thread that clearly explains why we
    > should want this exact thing. I don't want to make it sound like I
    > know that this is useless; I'm sure that Guillaume probably has lots
    > of hands-on tuning experience with this stuff that I lack. But the
    > reasons aren't clearly spelled out as far as I can see, and I'm having
    > some trouble imagining what they are.
    
    Thanks for the summary.  My main worry is that these are kind of hard
    to act on for tuning when aggregated at relation level (Guillaume,
    feel free to counter-argue!).  The main point that comes into mind is
    that single table scans would be mostly involved with OLTP workloads
    or simple joins, where parallel workers are of little use.  That could
    be much more interesting for analytical-ish workloads with more
    complex plan pattern where one or more Gather or GatherMerge nodes are
    involved.  Still, even in this case I suspect that most users will
    finish by looking at plan patterns, and that these counters added for
    index or tables would have a limited impact at the end.
    --
    Michael
    

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

    Bertrand Drouvot <bertranddrouvot.pg@gmail.com> — 2026-01-09T07:53:08Z

    Hi,
    
    On Tue, Nov 12, 2024 at 12:41:19PM +0900, Michael Paquier wrote:
    > On Mon, Nov 11, 2024 at 11:06:43AM -0500, Robert Haas wrote:
    > > But it is unclear to me what sort of tuning we would do based on
    > > knowing how many of the scans on a certain table or a certain index
    > > were parallel vs non-parallel. I have not fully reviewed the threads
    > > linked in the original post; but I did look at them briefly and did
    > > not immediately see discussion of the specific counters proposed here.
    > > I also don't see anything in this thread that clearly explains why we
    > > should want this exact thing. I don't want to make it sound like I
    > > know that this is useless; I'm sure that Guillaume probably has lots
    > > of hands-on tuning experience with this stuff that I lack. But the
    > > reasons aren't clearly spelled out as far as I can see, and I'm having
    > > some trouble imagining what they are.
    > 
    > Thanks for the summary.  My main worry is that these are kind of hard
    > to act on for tuning when aggregated at relation level (Guillaume,
    > feel free to counter-argue!).  The main point that comes into mind is
    > that single table scans would be mostly involved with OLTP workloads
    > or simple joins, where parallel workers are of little use.  That could
    > be much more interesting for analytical-ish workloads with more
    > complex plan pattern where one or more Gather or GatherMerge nodes are
    > involved.  Still, even in this case I suspect that most users will
    > finish by looking at plan patterns, and that these counters added for
    > index or tables would have a limited impact at the end.
    
    While working on flushing stats outside of transaction boundaries (patch not
    shared yet but linked to [1]), I realized that parallel workers could lead to
    incomplete and misleading statistics. Indeed, they update "their" relation
    stats during their shutdown regardless of the "main" transaction status. 
    
    It means that, for example, stats like seq_scan, last_seq_scan and seq_tup_read
    are updated by the parallel workers during their shutdown while the main
    transaction has not finished. The stats are then somehow incomplete because the main
    worker has not updated its stats yet. I think that could lead to misleading stats
    that a patch like this one could help to address. For example, parallel workers
    could update parallel_* dedicated stats and leave the non parallel_* stats update
    responsibility to the main worker when the transaction finishes. That would make
    the non parallel_* stats consistent whether parallel workers are used or not.
    
    Thoughts?
    
    [1]: https://www.postgresql.org/message-id/aVvgJu0BhnmzBWZ1@ip-10-97-1-34.eu-west-3.compute.internal
    
    Regards,
    
    -- 
    Bertrand Drouvot
    PostgreSQL Contributors Team
    RDS Open Source Databases
    Amazon Web Services: https://aws.amazon.com
    
    
    
    

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

    Michael Paquier <michael@paquier.xyz> — 2026-01-19T00:25:59Z

    On Fri, Jan 09, 2026 at 07:53:08AM +0000, Bertrand Drouvot wrote:
    > While working on flushing stats outside of transaction boundaries (patch not
    > shared yet but linked to [1]), I realized that parallel workers could lead to
    > incomplete and misleading statistics. Indeed, they update "their" relation
    > stats during their shutdown regardless of the "main" transaction status. 
    > 
    > It means that, for example, stats like seq_scan, last_seq_scan and seq_tup_read
    > are updated by the parallel workers during their shutdown while the main
    > transaction has not finished. The stats are then somehow incomplete because the main
    > worker has not updated its stats yet. I think that could lead to misleading stats
    > that a patch like this one could help to address. For example, parallel workers
    > could update parallel_* dedicated stats and leave the non parallel_* stats update
    > responsibility to the main worker when the transaction finishes. That would make
    > the non parallel_* stats consistent whether parallel workers are used or not.
    
    (Re-reading the thread to remember the context..)
    
    It depends, I guess.  I still doubt that adding parallel worker data
    at table and index level is the right move compared to all the
    information we have now on HEAD, because this extra information is not
    actionable in terms of tuning GUCs or reloptions.
    
    Now, do you think that the extra noise of data flushed by the parallel
    workers shutting down and flushing their data before the main
    transaction has committed in the "main" backend process could really
    impact the tuning decisions users may want to take?  Stats are not
    about precision, they are about offering trends that help in taking 
    better decisions to drive the backend server in a direction where its
    administrator wants to lead it to.  If the noise is too high, and that
    this noise drives to incorrect tuning decision, the system could go
    crazy and that would be an issue.  My question is then: does this
    extra data flushed by the parallel workers before transaction end,
    which you are qualifying as noise, really matter when it comes to the
    tuning decisions one needs to take?  That stance would apply mostly to
    analytical queries, of course, where parallel workers would have more
    data to flush.  Parallel workers flushing could have a lot of data to
    report, but the transaction commit just delays the availability of
    this information.
    
    When it comes to what you are describing as problem, my intuition is
    telling me that we don't have a problem to solve at all here, but I'm
    OK to be proved wrong, as well. 
    --
    Michael