Date: Fri, 12 Jul 2019 16:10:16 +0900 (Tokyo Standard Time)
Message-Id: <20190712.161016.56338282.horikyota.ntt@gmail.com>
To: andres@anarazel.de
Cc: pgsql-hackers@postgresql.org
Subject: Re: Remove page-read callback from XLogReaderState.
From: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
In-Reply-To: <20190528114524.dvj6ymap2virlzro@alap3.anarazel.de>
References: <20190418.210257.43726183.horiguchi.kyotaro@lab.ntt.co.jp>
	<20190528114524.dvj6ymap2virlzro@alap3.anarazel.de>
User-Agent: Mew version 6.5 on Emacs 22.2 / Mule 5.0
 =?iso-2022-jp?B?KBskQjpnGyhCKQ==?= / Meadow-3.01-dev (TSUBO-SUMIRE)
Mime-Version: 1.0
Content-Type: Text/Plain; charset=us-ascii
Content-Transfer-Encoding: 7bit
Precedence: bulk

At Tue, 28 May 2019 04:45:24 -0700, Andres Freund <andres@anarazel.de> wrote in <20190528114524.dvj6ymap2virlzro@alap3.anarazel.de>
> Hi,
> 
> On 2019-04-18 21:02:57 +0900, Kyotaro HORIGUCHI wrote:
> > Hello. As mentioned before [1], read_page callback in
> > XLogReaderState is a cause of headaches. Adding another
> > remote-controlling stuff to xlog readers makes things messier [2].
> > 
> > I refactored XLOG reading functions so that we don't need the
> > callback. In short, ReadRecrod now calls XLogPageRead directly
> > with the attached patch set.
> > 
> > |     while (XLogReadRecord(xlogreader, RecPtr, &record, &errormsg)
> > |            == XLREAD_NEED_DATA)
> > |         XLogPageRead(xlogreader, fetching_ckpt, emode, randAccess);
> > 
> > On the other hand, XLogReadRecord became a bit complex. The patch
> > makes XLogReadRecord a state machine. I'm not confident that the
> > additional complexity is worth doing. Anyway I'll gegister this
> > to the next CF.
> 
> Just FYI, to me this doesn't clearly enough look like an improvement,
> for a change of this size.

Thanks for the opiniton. I kinda agree about size but it is a
decision between "having multiple callbacks called under the
hood" vs "just calling a series of functions".  I think the
patched XlogReadRecord is easy to use in many situations.

It would be better if I could completely refactor the function
without the syntax tricks but I think the current patch is still
smaller and clearer than overhauling it.

If many of the folks think that adding a callback is better than
this refactoring, I will withdraw this..

reagrds.

-- 
Kyotaro Horiguchi
NTT Open Source Software Center