Re: WAL usage calculation patch

On Thu, Apr 2, 2020 at 6:18 PM Julien Rouhaud <rjuju123@gmail.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 */"?

If you agree, then a similar comment exists in
v11-0001-Add-infrastructure-to-track-WAL-usage, consider changing that
as well.


v11-0002-Add-option-to-report-WAL-usage-in-EXPLAIN-and-au
5.
Specifically, include the
+      number of records, full page images and bytes generated.

How about making the above slightly clear?  "Specifically, include the
number of records, number of full page image records and amount of WAL
bytes generated.

-- 
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com