Re: WAL consistency check facility

Hi Kuntal,

Thanks for the patch.

Current patch has no docs, no tests and no explanatory comments, so
makes review quite hard.

The good news is you might discover a few bugs with it, so its worth
pursuing actively in this CF, though its not near to being
committable.

I think you should add this as part of the default testing for both
check and installcheck. I can't imagine why we'd have it and not use
it during testing.


On 25 August 2016 at 18:41, Kuntal Ghosh <kuntalghosh.2007@gmail.com> wrote:

> * wal_consistency_mask = 511  /* Enable consistency check mask bit*/

What does this mean? (No docs)


> What needs to be done:
> 1. Add support for other Resource Managers.

We probably need to have a discussion as to why you think this should
be Rmgr dependent?
Code comments would help there.

If it does, then you should probably do this by extending RmgrTable
with an rm_check, so you can call it like this...

RmgrTable[record->xl_rmid].rm_check

I'm interested in how we handle the new generic WAL format for blocks.
Surely if we can handle that then we won't need an Rmgr dependency?
I'm sure you have reasons, they just need to be explained long hand -
don't assume anything.


> 5. Generalize the page type identification technique.

Why not do this first?

There are some coding guideline stuff to check as well.

Thanks

-- 
Simon Riggs                http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services