Re: WAL usage calculation patch

Julien Rouhaud <rjuju123@gmail.com>

From: Julien Rouhaud <rjuju123@gmail.com>
To: Amit Kapila <amit.kapila16@gmail.com>
Cc: Kirill Bychik <kirill.bychik@gmail.com>, PostgreSQL Hackers <pgsql-hackers@postgresql.org>, Fujii Masao <masao.fujii@oss.nttdata.com>, Thomas Munro <thomas.munro@gmail.com>
Date: 2020-03-28T13:38:27Z
Lists: pgsql-hackers

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →
  1. Change the display of WAL usage statistics in Explain.

  2. Cosmetic fixups for WAL usage work.

  3. Allow parallel create index to accumulate buffer usage stats.

  4. Allow autovacuum to log WAL usage statistics.

  5. Add the option to report WAL usage in EXPLAIN and auto_explain.

  6. Allow pg_stat_statements to track WAL usage statistics.

  7. Add infrastructure to track WAL usage.

  8. Include information on buffer usage during planning phase, in EXPLAIN output, take two.

  9. Include information on buffer usage during planning phase, in EXPLAIN output.

  10. Allow parallel vacuum to accumulate buffer usage.

  11. Allow pg_stat_statements to track planning statistics.

On Sat, Mar 28, 2020 at 04:14:04PM +0530, Amit Kapila wrote:
> 
> I see some basic problems with the patch.  The way it tries to compute
> WAL usage for parallel stuff doesn't seem right to me.  Can you share
> or point me to any test done where we have computed WAL for parallel
> operations like Parallel Vacuum or Parallel Create Index?

Ah, that's indeed a good point and AFAICT WAL records from parallel utility
workers won't be accounted for.  That being said, I think that an argument
could be made that proper infrastructure should have been added in the original
parallel utility patches, as pg_stat_statement is already broken wrt. buffer
usage in parallel utility, unless I'm missing something.

> Basically,
> I don't know changes done in ExecInitParallelPlan and friends allow us
> to compute WAL for parallel operations.  Those will primarily cover
> parallel queries that won't write WAL.  How you have tested those
> changes?

I didn't tested those, and I'm not even sure how to properly and reliably test
that.  Do you have any advice on how to achieve that?

However the patch is mimicking the buffer instrumentation that already exists,
and the approach also looks correct to me.  Do you have a reason to believe
that the approach that works for buffer usage wouldn't work for WAL records? (I
of course agree that this should be tested anyway)