pg.ddx.io  pgsql-hackers@postgresql.org mailing list archive  

help / color / mirror / Atom feed
From: Julien Rouhaud <rjuju123@gmail.com>
To: Amit Kapila <amit.kapila16@gmail.com>
Cc: Kirill Bychik <kirill.bychik@gmail.com>
Cc: PostgreSQL Hackers <pgsql-hackers@postgresql.org>
Cc: Fujii Masao <masao.fujii@oss.nttdata.com>
Cc: Thomas Munro <thomas.munro@gmail.com>
Subject: Re: WAL usage calculation patch
Date: Sat, 28 Mar 2020 14:38:27 +0100
Message-ID: <20200328133827.GA12854@nol> (raw)
In-Reply-To: <CAA4eK1+nLPguL+a1JStyx+L9xh0X3nQ3tXptorsggT6dn9oPLQ@mail.gmail.com>
References: <20200318171916.GA69354@nol>
	<5b71ad52-68c6-08cb-6408-31307e9e6f10@oss.nttdata.com>
	<20200319153129.GA36079@nol>
	<CAB-hujo_bnDsV7C1BNLMacC4gKTyHeR8_Fsdyv+rSoOcsk5HiA@mail.gmail.com>
	<a1452be6-9e82-59c4-3c54-b3f8919e75c0@oss.nttdata.com>
	<762d4ee8-9710-cdcd-e661-b5c79aaa9bf0@oss.nttdata.com>
	<CAOBaU_bb6gf-33qCsMiZYy6EupqAcT5xga3iM8q-=-Ap=+9w_w@mail.gmail.com>
	<CAB-hujrX8BWGYDH0h-4c0AQt0dfkNdaOPAaw5n7KWnzVe6vPOQ@mail.gmail.com>
	<CAOBaU_bRp3HmAnerZWFO00f8dx=nwU5WMUVY0woVY5kwMa_jEg@mail.gmail.com>
	<CAA4eK1+nLPguL+a1JStyx+L9xh0X3nQ3tXptorsggT6dn9oPLQ@mail.gmail.com>

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)





view thread (130+ messages)  latest in thread

Message-ID: <20200328133827.GA12854@nol>
Permalink:  ../20200328133827.GA12854@nol/
Also on:    postgresql.org/message-id/20200328133827.GA12854@nol

 · 

reply
Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: pgsql-hackers@postgresql.org
  Cc: rjuju123@gmail.com, amit.kapila16@gmail.com, kirill.bychik@gmail.com, masao.fujii@oss.nttdata.com, thomas.munro@gmail.com
  Subject: Re: WAL usage calculation patch
  In-Reply-To: <20200328133827.GA12854@nol>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox