Re: [HACKERS] [PATCH] Add pg_disable_checksums() and supportinginfrastructure

> On Feb 19, 2017, at 5:05 AM, Robert Haas <robertmhaas@gmail.com> wrote:
>
> On Fri, Feb 17, 2017 at 2:28 AM, David Christensen <david@endpoint.com> wrote:
>> - Change "data_checksums" from a simple boolean to "data_checksum_state", an enum type for all of
>>  the potentially-required states for this feature (as well as enabling).
>
> Color me skeptical.  I don't know what CHECKSUMS_ENABLING,
> CHECKSUMS_ENFORCING, and CHECKSUMS_REVALIDATING are intended to
> represent -- and there's no comments in the patch explaining it -- but
> if we haven't yet written the code to enable checksums, how do we know
> for sure which states it will require?
>
> If we're going to accept a patch to disable checksums without also
> having the capability to enable checksums, I think we should leave out
> the speculative elements about what might be needed on the "enable"
> side and just implement the minimal "disable" side.
>
> However, FWIW, I don't accept that being able to disable checksums
> online is a sufficient advance to justify enabling checksums by
> default.  Tomas had some good points on another thread about what
> might be needed to really make that a good choice, and I'm still
> skeptical about whether checksums catch any meaningful number of
> errors that wouldn't be caught otherwise, and about the degree to
> which any complaints it issues are actionable.  I'm not really against
> this patch on its own merits, but I think it's a small advance in an
> area that needs a lot of work.  I think it would be a lot more useful
> if we had a way to *enable* checksums online.  Then people who find
> out that checksums exist and want them have an easier way of getting
> them, and anyone who uses the functionality in this patch and then
> regrets it has a way to get back.


Hi Robert, this is part of a larger patch which *does* enable the checksums online; I’ve been extracting the necessary
piecesout with the understanding that some people thought the disable code might be useful in its own merit.  I can add
documentationfor the various states.  The CHECKSUM_REVALIDATE is the only one I feel is a little wibbly-wobbly; the
otherstates are required because of the fact that enabling checksums requires distinguishing between “in process of
enabling”and “everything is enabled”. 

My design notes for the patch were submitted to the list with little comment; see:
https://www.postgresql.org/message-id/1E6E64E9-634B-43F4-8AA2-CD85AD92D2F8%40endpoint.com

I have since added the WAL logging of checksum states, however I’d be glad to take feedback on the other proposed
approaches(particularly the system catalog changes + the concept of a checksum cycle).] 

Best,

David
--
David Christensen
End Point Corporation
david@endpoint.com
785-727-1171