Re: Remove page-read callback from XLogReaderState.

From: Kyotaro HORIGUCHI <horiguchi(dot)kyotaro(at)lab(dot)ntt(dot)co(dot)jp>
To: ah(at)cybertec(dot)at
Cc: pgsql-hackers(at)postgresql(dot)org
Subject: Re: Remove page-read callback from XLogReaderState.
Date: 2019-05-24 02:56:24
Message-ID: 20190524.115624.41405730.horiguchi.kyotaro@lab.ntt.co.jp
Views: Raw Message | Whole Thread | Download mbox
Thread:
Lists: pgsql-hackers

Thank you for looking this, Antonin.

At Wed, 22 May 2019 13:53:23 +0200, Antonin Houska <ah(at)cybertec(dot)at> wrote in <25494(dot)1558526003(at)spoje(dot)net>
> Kyotaro HORIGUCHI <horiguchi(dot)kyotaro(at)lab(dot)ntt(dot)co(dot)jp> wrote:
>
> > Hello. Thank you for looking this.
> > ...
> > Yeah, I'll register this, maybe the week after next week.
>
> I've checked the new version. One more thing I noticed now is that XLR_STATE.j
> is initialized to zero, either by XLogReaderAllocate() which zeroes the whole
> reader state, or later by XLREAD_RESET. This special value then needs to be
> handled here:
>
> #define XLR_SWITCH() \
> do { \
> if ((XLR_STATE).j) \
> goto *((void *) (XLR_STATE).j); \
> XLR_CASE(XLR_INIT_STATE); \
> } while (0)
>
> I think it's better to set the label always to (&&XLR_INIT_STATE) so that
> XLR_SWITCH can perform the jump unconditionally.

I thought the same but did not do that since label is
function-scoped so it cannot be referred outside the defined
function.

I moved the state variable from XLogReaderState into functions
static variable. It's not problem since the functions are
non-reentrant in the first place.

> Attached is also an (unrelated) comment fix proposal.

Sounds reasoable. I found another typo "acutually" there.

- int32 readLen; /* bytes acutually read, must be larger than
+ int32 readLen; /* bytes acutually read, must be at least

I fixed it with other typos found.

v3-0001 : Changed macrosas suggested.

v3-0002, 0004: Fixed comments. Fixes following changes of
macros. Renamed state symbols.

v3-0003, 0005-0010: No substantial change from v2.

regards.

--
Kyotaro Horiguchi
NTT Open Source Software Center

Attachment Content-Type Size
v3-0002-Make-ReadPageInternal-a-state-machine.patch text/x-patch 24.1 KB
v3-0003-Change-interface-of-XLogReadRecord.patch text/x-patch 12.0 KB
v3-0004-Make-XLogReadRecord-a-state-machine.patch text/x-patch 16.5 KB
v3-0005-Make-XLogPageRead-not-use-callback-but-call-the-func.patch text/x-patch 4.7 KB
v3-0006-Make-XlogReadTwoPhaseData-not-use-callback-but-call-.patch text/x-patch 4.0 KB
v3-0007-Make-logical-rep-stuff-not-use-callback-but-call-the.patch text/x-patch 9.2 KB
v3-0008-Make-pg_waldump-not-use-callback-but-call-the-functi.patch text/x-patch 5.6 KB
v3-0009-Make-pg_rewind-not-use-callback-but-call-the-functio.patch text/x-patch 6.3 KB
v3-0010-Remove-callback-entry-from-XLogReaderState.patch text/x-patch 7.5 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message David Rowley 2019-05-24 04:37:35 Re: Should we warn against using too many partitions?
Previous Message David Rowley 2019-05-24 02:47:56 Re: Top-N sorts in EXPLAIN, row count estimates, and parallelism