Re: WAL usage calculation patch

On Wed, Apr 1, 2020 at 8:00 PM Julien Rouhaud <rjuju123(at)gmail(dot)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