On Thu, Nov 3, 2016 at 3:24 PM, Kuntal Ghosh <kuntalghosh.2...@gmail.com> wrote: > On Thu, Nov 3, 2016 at 2:35 AM, Michael Paquier > <michael.paqu...@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.
Wouldn't the definition of a new redo action make sense then? Say SKIPPED. None of the existing actions match the non-apply case. -- Michael -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers