On 27 August 2016 at 12:09, Kuntal Ghosh <kuntalghosh.2007@gmail.com> wrote:
>>> * wal_consistency_mask = 511 /* Enable consistency check mask bit*/
>>
>> What does this mean? (No docs)
>
> I was using this parameter as a masking integer to indicate the
> operations(rmgr list) for which we need this feature to be enabled.
> Since, this could be confusing, I've changed it accordingly so that it
> accepts a list of rmgrIDs. (suggested by Michael, Amit and Robert)
Why would we want that?
>>> 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
>
> +1.
> I'm modifying it accordingly. I'm calling this function after
> RmgrTable[record->xl_rmid].rm_redo.
>
>>> 5. Generalize the page type identification technique.
>>
>> Why not do this first?
>>
>
> At present, I'm using special page size and page ID to identify page
> type. But, I've noticed some cases where the entire page is
> initialized to zero (Ex: hash_xlog_squeeze_page). RmgrID and info bit
> can help us to identify those pages.
I'd prefer a solution that was not dependent upon RmgrID at all.
If there are various special cases that we need to cater for, ISTM
they would be flaws in the existing WAL implementation rather than
anything we would want to perpetuate. I hope we'll spend time fixing
them rather than add loads of weird code to work around the
imperfections.
Underdocumented special case code is going to be unbelievably
difficult to get right in the long term.
--
Simon Riggs http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services