Date: Sun, 29 Mar 2020 14:31:41 +0200
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>
Subject: Re: WAL usage calculation patch
Message-ID: <20200329123141.GB79261@nol>
References: <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>
 <20200328133827.GA12854@nol>
 <CAA4eK1Jzex_fOB7N7aPwDf1=QoT05cxwxxZs_eW9DtqbB1EspA@mail.gmail.com>
 <20200329075549.6l54fcjfh73rxufb@nol>
 <CAA4eK1+YTjHmK3_4carNz=3bmx2+H4TX=EZj87hk_WL-gV3a8Q@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: 
 <CAA4eK1+YTjHmK3_4carNz=3bmx2+H4TX=EZj87hk_WL-gV3a8Q@mail.gmail.com>
Precedence: bulk

Hi Amit,

Sorry I just noticed your mail.

On Sun, Mar 29, 2020 at 05:12:16PM +0530, Amit Kapila wrote:
> On Sun, Mar 29, 2020 at 1:26 PM Julien Rouhaud <rjuju123@gmail.com> wrote:
> >
> > I'm not sure that I get your point.  I'm assuming that you meant
> > parallel-read-only queries, but surely buffer usage infrastructure for
> > parallel query relies on the same approach as non-parallel one (each node
> > computes the process-local pgBufferUsage diff) and sums all of that at the end
> > of the parallel query execution.  I also don't see how whether the query is
> > read-only or not is relevant here as far as instrumentation is concerned,
> > especially since read-only query can definitely do writes and increase the
> > count of dirtied buffers, like a write query would.  For instance a hint
> > bit change can be done in a parallel query AFAIK, and this can generate WAL
> > records in wal_log_hints is enabled, so that's probably one way to test it.
> >
> 
> Yeah, that way we can test it.  Can you try that?
> 
> > I now think that not adding support for WAL buffers in EXPLAIN output in the
> > initial patch scope was a mistake, as this is probably the best way to test the
> > WAL counters for parallel queries.  This shouldn't be hard to add though, and I
> > can work on it quickly if there's still a chance to get this feature included
> > in pg13.
> >
> 
> I am not sure we will add it in Explain or not (maybe we need inputs
> from others in this regard), but if it helps in testing this part of
> the patch, then it is a good idea to write a patch for it.  You might
> want to keep it separate from the main patch as we might not commit
> it.

As I just wrote in [1] that's exactly what I did.  Using parallel query and
concurrent update on a table I could see that WAL usage for parallel query
seems to be working as one could expect.

> Sure, I am fine with that but I am not sure if it is a good idea to
> commit this patch without having a way to compute WAL utilization for
> those commands.

I'm generally fine with waiting for a fix for the existing issue to be
committed.  But as the feature freeze is approaching, I hope that it won't mean
postponing this feature to v14 because a related 2yo bug has just been
discovered, as it would seem a bit unfair.