Re: WAL usage calculation patch

On Thu, Apr 2, 2020 at 6:41 PM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
>
> On Thu, Apr 2, 2020 at 6:18 PM Julien Rouhaud <rjuju123(at)gmail(dot)com> wrote:
> >
> > =# select query, calls, wal_bytes, wal_records, wal_num_fpw from pg_stat_statements where query ilike '%create index%';
> > query | calls | wal_bytes | wal_records | wal_num_fpw
> > ----------------------------------------------+-------+-----------+-------------+-------------
> > create index t1_idx_parallel_0 ON t1(id) | 1 | 20389743 | 2762 | 2758
> > create index t1_idx_parallel_0_bis ON t1(id) | 1 | 20394391 | 2762 | 2758
> > create index t1_idx_parallel_0_ter ON t1(id) | 1 | 20395155 | 2762 | 2758
> > create index t1_idx_parallel_1 ON t1(id) | 1 | 20388335 | 2762 | 2758
> > create index t1_idx_parallel_2 ON t1(id) | 1 | 20389091 | 2762 | 2758
> > create index t1_idx_parallel_3 ON t1(id) | 1 | 20389847 | 2762 | 2758
> > create index t1_idx_parallel_4 ON t1(id) | 1 | 20390603 | 2762 | 2758
> > create index t1_idx_parallel_5 ON t1(id) | 1 | 20391359 | 2762 | 2758
> > create index t1_idx_parallel_6 ON t1(id) | 1 | 20392115 | 2762 | 2758
> > create index t1_idx_parallel_7 ON t1(id) | 1 | 20392871 | 2762 | 2758
> > create index t1_idx_parallel_8 ON t1(id) | 1 | 20393627 | 2762 | 2758
> > (11 rows)
> >
> > =# select relname, pg_relation_size(oid) from pg_class where relname like '%t1_id%';
> > relname | pg_relation_size
> > -----------------------+------------------
> > t1_idx_parallel_0 | 22487040
> > t1_idx_parallel_0_bis | 22487040
> > t1_idx_parallel_0_ter | 22487040
> > t1_idx_parallel_2 | 22487040
> > t1_idx_parallel_1 | 22487040
> > t1_idx_parallel_4 | 22487040
> > t1_idx_parallel_3 | 22487040
> > t1_idx_parallel_5 | 22487040
> > t1_idx_parallel_6 | 22487040
> > t1_idx_parallel_7 | 22487040
> > t1_idx_parallel_8 | 22487040
> > (9 rows)
> >
> >
> > So while the number of WAL records and full page images stay constant, we can
> > see some small fluctuations in the total amount of generated WAL data, even for
> > multiple execution of the sequential create index. I'm wondering if the
> > fluctuations are due to some other internal details or if the WalUsage support
> > is just completely broken (although I don't see any obvious issue ATM).
> >
>
> I think we need to know the reason for this. Can you try with small
> size indexes and see if the problem is reproducible? If it is, then it
> will be easier to debug the same.
>
> Few other minor comments
> ------------------------------------
> pg_stat_statements patch
> 1.
> +--
> +-- CRUD: INSERT SELECT UPDATE DELETE on test non-temp table to
> validate WAL generation metrics
> +--
>
> The word 'non-temp' in the above comment appears out of place. We
> don't need to specify it.
>
> 2.
> +-- SELECT usage data, check WAL usage is reported, wal_records equal
> rows count for INSERT/UPDATE/DELETE
> +SELECT query, calls, rows,
> +wal_bytes > 0 as wal_bytes_generated,
> +wal_records > 0 as wal_records_generated,
> +wal_records = rows as wal_records_as_rows
> +FROM pg_stat_statements ORDER BY query COLLATE "C";
>
> The comment doesn't seem to match what we are doing in the statement.
> I think we can simplify it to something like "check WAL is generated
> for above statements:
>
> 3.
> @@ -185,6 +185,9 @@ typedef struct Counters
> int64 local_blks_written; /* # of local disk blocks written */
> int64 temp_blks_read; /* # of temp blocks read */
> int64 temp_blks_written; /* # of temp blocks written */
> + uint64 wal_bytes; /* total amount of WAL bytes generated */
> + int64 wal_records; /* # of WAL records generated */
> + int64 wal_num_fpw; /* # of WAL full page image generated */
> double blk_read_time; /* time spent reading, in msec */
> double blk_write_time; /* time spent writing, in msec */
> double usage; /* usage factor */
>
> It is better to keep wal_bytes should be after wal_num_fpw as it is in
> the main patch. Also, consider changing at other places in this
> patch. I think we should add these new fields after blk_write_time or
> at the end after usage.
>
> 4.
> /* # of WAL full page image generated */
> Can we change it to "/* # of WAL full page image records generated */"?

IMHO, "# of WAL full-page image records" seems like the number of wal
record which contains the full-page image. But, actually, this is the
total number of the full-page images, not the number of records that
have a full-page image.

--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com