Re: WAL format and API changes (9.5)

On 2014-08-14 12:41:35 +0300, Heikki Linnakangas wrote:
> On 08/14/2014 10:29 AM, Michael Paquier wrote:
> >On Thu, Aug 14, 2014 at 3:04 AM, Alvaro Herrera
> ><alvherre(at)2ndquadrant(dot)com> wrote:
> >>Heikki Linnakangas wrote:
> >>What's with XLogReplayLSN and XLogReplayRecord? IMO the xlog code has
> >>more than enough global variables already, it'd be good to avoid two
> >>more if possible. Is there no other good way to get this info down to
> >>whatever it is that needs them?
> >Yep, they do not look necessary. Looking at the patch, you could get
> >rid of XLogReplayLSN and XLogReplayRecord by adding two extra argument
> >to XLogReplayBuffer: one for the LSN of the current record, and a
> >second for the record pointer. The code just saves those two variables
> >in the redo loop of StartupXLOG to only reuse them in
> >XLogReplayBufferExtended, and I saw no code paths in the redo routines
> >where XLogReplayBuffer is called at places without the LSN position
> >and the record pointer.
> >
> >However I think that Heikki introduced those two variables to make the
> >move to the next patch easier.
>
> The next patch doesn't necessary require them either, you could always pass
> the LSN and record as an argument. I wanted to avoid that, because every
> redo function would just pass the current record being replayed, so it seems
> nicer to pass that information "out-of-band". I guess if we do that, though,
> we should remove those arguments from rm_redo interface altogether, and
> always rely on the global variables to get the "current" record or its LSN.
> I'm not wedded on this, I could be persuaded to go either way...

I personally find the out of band transport really ugly.

Greetings,

Andres Freund

--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services