On Thu, Nov 3, 2016 at 2:35 AM, Michael Paquier
<michael.paquier@gmail.com> wrote:
>> - Another suggestion was to remove wal_consistency from PostgresNode.pm
>> because small buildfarm machines may suffer on it. Although I've no
>> experience in this matter, I would like to be certain that nothings breaks
>> in recovery tests after some modifications.
>
> An extra idea that I have here would be to extend the TAP tests to
> accept an environment variable that would be used to specify extra
> options when starting Postgres instances. Buildfarm machines could use
> it.
>
It can be added as a separate feature.
>
> - /* If it's a full-page image, restore it. */
> - if (XLogRecHasBlockImage(record, block_id))
> + /* If full-page image should be restored, do it. */
> + if (XLogRecBlockImageApply(record, block_id))
> Hm. It seems to me that this modification is incorrect. If the page
> does not need to be applied, aka if it needs to be used for
> consistency checks, what should be done is more something like the
> following in XLogReadBufferForRedoExtended:
> if (Apply(record, block_id))
> return;
> if (HasImage)
> {
> //do what needs to be done with an image
> }
> else
> {
> //do default things
> }
>
XLogReadBufferForRedoExtended should return a redo action
(block restored, done, block needs redo or block not found). So, we
can't just return
from the function if BLKIMAGE_APPLY flag is not set. It still has to
check whether a
redo is required or not.
> XLogRecBlockImageApply() should only check for BKP_APPLY and not imply
> HasImage(). This will be more flexible when for example using it for
> assertions.
seems reasonable.
> It would be more consistent to have a boolean flag to treat
> BKPIMAGE_APPLY in xlogreader.c. Have a look at has_image for example.
For flags in bimg_info, we directly check if the mask bit is set in bimg_info
(ex: hole, compressed). Besides, we use this flag only at
XLogReadBufferForRedoExtended.
--
Thanks & Regards,
Kuntal Ghosh
EnterpriseDB: http://www.enterprisedb.com