Re: preserving forensic information when we freeze

On Thu, Dec 19, 2013 at 3:36 PM, Alvaro Herrera
<alvherre@2ndquadrant.com> wrote:
> Robert Haas escribió:
>> I think it may have been a mistake to divide responsibility between
>> the prepare and execute functions the way we did in this case, because
>> it doesn't appear to be a clean separation of concerns.  But it's not
>> this patch's job to kibitz that decision, so this version just fits in
>> with the way things are already being done there.
>
> If you want to change how it works, feel free to propose a patch; I'm
> not in love with what we're doing, honestly, and I did propose the idea
> of using some flag bits instead of the whole mask, but didn't get any
> traction.  (Not that that thread had a lot of participants.)
>
> Or are you suggesting that I should do it?  I would have welcomed
> feedback when I was on that, but I have moved on to other things now,
> and I don't want to be a blocker for the forensic freeze patch.
>
> Anyway I think if we want to change it, the time is now, before we
> release 9.3.3.

I am sorry I wasn't able to follow that thread in more detail at the
time, and I'm not trying to create extra work for you now.  You've put
a lot of work into stabilizing the fkeylocks stuff and it's not my
purpose to cast aspersions on that, nor do I think that this is so bad
we can't live with it.  The somewhat ambiguous phrasing of that email
is attributable to the fact that I really don't have a clear idea what
would be better than what you've got here now, and even if I did, I'm
not eager to be the guy who insists on refactoring and breaks things
again in the process.

It's tempting to think that the prepare function log a set of flags
indicating what logical operations should be performed on the target
tuple, rather than the new infomask and infomask2 fields per se; or
else that we ought to just log the whole HeapTupleHeader, so that the
execute function just copies the data in, splat.  In other words, make
the logging either purely logical, or purely physical, not a mix.  But
making it purely logical would involve translating between multiple
sets of flags in a way that might not accomplish much beyond
increasing the possibility for error, and making it purely physical
would make the WAL record bigger for no real gain.  So perhaps the way
you've chosen is best after all, despite my reservations.

But in either case, it was not my purpose to hijack this thread to
talk about that patch, just to explain how I've updated this patch to
(hopefully) satisfy Andres's concerns.

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