Re: WAL usage calculation patch

On Wed, Apr 1, 2020 at 8:00 PM Julien Rouhaud <rjuju123@gmail.com> wrote:
>
> On Wed, Apr 01, 2020 at 04:29:16PM +0530, Amit Kapila wrote:
> > 3. Doing some testing with and without parallelism to ensure WAL usage
> > data is correct would be great and if possible, share the results?
>
>
> I just saw that Dilip did some testing, but just in case here is some
> additional one
>
> - vacuum, after a truncate, loading 1M row and a "UPDATE t1 SET id = id"
>
> =# select query, calls, wal_bytes, wal_records, wal_num_fpw from pg_stat_statements where query ilike '%vacuum%';
>          query          | calls | wal_bytes | wal_records | wal_num_fpw
> ------------------------+-------+-----------+-------------+-------------
>  vacuum (parallel 3) t1 |     1 |  20098962 |       34104 |           2
>  vacuum (parallel 0) t1 |     1 |  20098962 |       34104 |           2
> (2 rows)
>
> - create index, overload t1's parallel_workers, using the 1M line just
>   vacuumed:
>
> =# alter table t1 set (parallel_workers = 2);
> ALTER TABLE
>
> =# create index t1_parallel_2 on t1(id);
> CREATE INDEX
>
> =# alter table t1 set (parallel_workers = 0);
> ALTER TABLE
>
> =# create index t1_parallel_0 on t1(id);
> CREATE INDEX
>
> =# 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_parallel_0 on t1(id) |     1 |  20355540 |        2762 |        2745
>  create index t1_parallel_2 on t1(id) |     1 |  20406811 |        2762 |        2758
> (2 rows)
>
> It all looks good to me.
>

Here the wal_num_fpw and wal_bytes are different between parallel and
non-parallel versions.  Is it due to checkpoint or something else?  We
can probably rule out checkpoint by increasing checkpoint_timeout and
other checkpoint related parameters.

>
> > 5.
> > -SELECT query, calls, rows FROM pg_stat_statements ORDER BY query COLLATE "C";
> > -               query               | calls | rows
> > ------------------------------------+-------+------
> > - SELECT $1::TEXT                   |     1 |    1
> > - SELECT PLUS_ONE($1)               |     2 |    2
> > - SELECT PLUS_TWO($1)               |     2 |    2
> > - SELECT pg_stat_statements_reset() |     1 |    1
> > +SELECT query, calls, rows, wal_bytes, wal_records FROM
> > pg_stat_statements ORDER BY query COLLATE "C";
> > +               query               | calls | rows | wal_bytes | wal_records
> > +-----------------------------------+-------+------+-----------+-------------
> > + SELECT $1::TEXT                   |     1 |    1 |         0 |           0
> > + SELECT PLUS_ONE($1)               |     2 |    2 |         0 |           0
> > + SELECT PLUS_TWO($1)               |     2 |    2 |         0 |           0
> > + SELECT pg_stat_statements_reset() |     1 |    1 |         0 |           0
> >  (4 rows)
> >
> > Again, I am not sure if these modifications make much sense?
>
>
> Those are queries that were previously executed.  As those are read-only query,
> that are pretty much guaranteed to not cause any WAL activity, I don't see how
> it hurts to test at the same time that that's we indeed record with
> pg_stat_statements, just to be safe.
>

On a similar theory, one could have checked bufferusage stats as well.
The statements are using some expressions so don't see any value in
check all usage data for such statements.

>  Once again, feel free to drop the extra
> wal_* columns from the output if you disagree.
>

Right now, that particular patch is not getting applied (probably due
to recent commit 17e0328224).  Can you rebase it?

>
>
> > v9-0004-Add-option-to-report-WAL-usage-in-EXPLAIN-and-aut
> >
> > 3.
> > + if (usage->wal_num_fpw > 0)
> > + appendStringInfo(es->str, " full page records=%ld",
> > +    usage->wal_num_fpw);
> > + if (usage->wal_bytes > 0)
> > + appendStringInfo(es->str, " bytes=" UINT64_FORMAT,
> > +    usage->wal_bytes);
> >
> > Shall we change to 'full page writes' or 'full page image' instead of
> > full page records?
>
>
> Indeed, I changed it in the (auto)vacuum output but missed this one.  Fixed.
>

I don't see this change in the patch.


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