Re: WAL consistency check facility

On Thu, Sep 1, 2016 at 4:12 PM, Simon Riggs <simon@2ndquadrant.com> wrote:
> So in my current understanding, a hinted change has by definition no
> WAL record, so we just ship a FPW.

Hmm.  An FPW would have to be contained in a WAL record, so it can't
be right to say that we ship an FPW for lack of a WAL record.  I think
what we ship is nothing at all when wal_log_hints is disabled, and
when wal_log_hints is enabled we log an FDW once per checkpoint.

> A non-hint change has a WAL record
> and it is my (possibly naive) hope that all changes to a page are
> reflected in the WAL record/replay,

I hope for this, too.

> so we can just make a simple
> comparison without caring what is the rmgr of the WAL record.

Sure, that is 100% possible, and likely a good idea as far as the
behavior on the standby is concerned.  What's not so clear is whether
a simple on/off switch is a wise plan on the master.

The purpose of this code, as I understand it, is to check for
discrepancies between "do" and "redo"; that is, to verify that the
changes made to the buffer at the time the WAL record is generated
produce the same result as replay of that WAL record on the standby.
To accomplish this purpose, a post-image of the affected buffers is
included in each and every WAL record.  On replay, that post-image can
be compared with the result of replay.  If they differ, PostgreSQL has
a bug.  I would not expect many users to run this in production,
because it will presumably be wicked expensive.  If I recall
correctly, doing full page writes once per buffer per checkpoint, the
space taken up by FPWs is >75% of WAL volume.  Doing it for every
record will be exponentially more expensive.  The primary audience of
this feature is PostgreSQL developers, who might want to use it to try
to verify that, for example, Amit's patch to add write-ahead logging
for hash indexes does not have bugs.[1]

Indeed, it had occurred to me that we might not even want to compile
this code into the server unless WAL_DEBUG is defined; after all, how
does it help a regular user to detect that the server has a bug?  Bug
or no bug, that's the code they've got.  But on further reflection, it
seems like it could be useful: if we suspect a bug in the redo code
but we can't reproduce it here, we could ask the customer to turn this
option on to see whether it produces logging indicating the nature of
the problem.  However, because of the likely expensive of enabling the
feature, it seems like it would be quite desirable to limit the
expense of generating many extra FPWs to the affected rmgr.  For
example, if a user has a table with a btree index and a gin index, and
we suspect a bug in GIN, it would be nice for the user to be able to
enable the feature *only for GIN* rather than paying the cost of
enabling it for btree and heap as well.[2]

Similarly, when we imagine a developer using this feature to test for
bugs, it may at times be useful to enable it across-the-board to look
for bugs in any aspect of the write-ahead logging system. However, at
other times, when the goal is to find bugs in a particular AM, it
might be useful to enable it only for the corresponding rmgr.  It is
altogether likely that this feature will slow the system down quite a
lot.  If enabling this feature for hash indexes also means enabling it
for the heap, the incremental performance hit might be sufficient to
mask concurrency-related bugs in the hash index code that would
otherwise have been found.  So, I think having at least some basic
filtering is probably a pretty smart idea.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

[1] It probably has bugs.
[2] One could of course add filtering considerably more complex than
per-rmgr - e.g. enabling it for only one particular relfilenode on a
busy production system might be rather desirable.  But I'm not sure we
really want to go there.  It adds a fair amount of complexity to a
feature that many people are obviously hoping will be quite simple to
use.