On Wed, Feb 8, 2017 at 1:25 AM, Kuntal Ghosh <kuntalghosh.2...@gmail.com> wrote: > Thank you Robert for the review. Please find the updated patch in the > attachment.
I have committed this patch after fairly extensive revisions: * Rewrote the documentation to give some idea what the underlying mechanism of operation of the feature is, so that users who choose to enable this will hopefully have some understanding of what they've turned on. * Renamed "char *page" arguments to "char *pagedata" and "Page page", because tempPage doesn't seem to be to be any better a name than page_norm. * Moved bufmask.c to src/backend/access/common, because there's no code in src/backend/storage/buffer that knows anything about the format of pages; that is the job of AMs, hence src/backend/access. * Improved some comments in bufmask.c * Removed consistencyCheck_is_enabled in favor of determining which RMs support masking by the presence of absence of an rm_mask function. * Removed assertion in checkXLogConsistency that consistency checking is enabled for the RM; that's trivially false if wal_consistency_checking is not the same on the master and the standby. (Note that quite apart from the issue of whether this function should exist at all, adding it to a header file after the closing #endif guard is certainly not right.) * Changed checkXLogConsistency to use RBM_NORMAL_NO_LOG instead of RBM_NORMAL. I'm not sure if there are any cases where this makes a difference, but it seems safer. * Changed checkXLogConsistency to skip pages whose LSN is newer than that of the record. Without this, if you shut down recovery and restart it, it complains of inconsistent pages and dies. (I'm not sure this is the only scenario that needs to be covered; it would be smart to do more testing of restarting the standby.) * Made wal_consistency_checking a developer option instead of a WAL option. Even though it CAN be used in production, we don't particularly want to encourage that; enabling WAL consistency checking has a big performance cost and makes your system more fragile not less -- a WAL consistency failure causes your standby to die a hard death. (Maybe there should be a way to suppress consistency checking on the standby -- but I think not just by requiring wal_consistency_checking on both ends. Or maybe we should just downgrade the FATAL to WARNING; blowing up the standby irrevocably seems like poor behavior.) * Coding style improvement in check_wal_consistency_checking. * Removed commas in messages added to pg_xlogdump; those didn't look good to me, on further review. * Comment improvements in xlog_internal.h and xlogreader.h I also bumped XLOG_PAGE_MAGIC (which is normally done by the committer, not the patch author, so I wasn't expecting that to be in the patch as submitted). -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
