Re: preserving forensic information when we freeze

On Tue, May 28, 2013 at 10:08 PM, Andres Freund <andres(at)2ndquadrant(dot)com> wrote:
> On 2013-05-28 21:26:49 -0400, Robert Haas wrote:
>> On Tue, May 28, 2013 at 8:00 PM, Andres Freund <andres(at)2ndquadrant(dot)com> wrote:
>> > I only suggested MOVED_IN/OFF because I didn't remember
>> > HEAP_XMIN_COMMITTED|HEAP_XMIN_INVALID was still free ;). So, unless that
>> > combination could have been produced in the past in a way I couldn't
>> > find so far, I am all for it. I don't see a problem with breaking
>> > backward compatibility on that level and I don't think there's any way
>> > to get there without some level of low level compat break.
>>
>> I'm not sure why you call it a compatibility break. It's true that an
>> old PostgreSQL can't read new heaps, but we've never guaranteed that
>> direction anyway, and every time we add or reinterpret any infomask
>> bits, that's true. For example, the fklocks patch did tat. What's
>> important is that a new PostgreSQL will still be able to read old
>> heaps; that is, pg_upgrade compatibility is preserved.
>
> Oh, not the on-disk format. But as you said, code that looks at xmin is
> going to need to change. fklocks broke code that relied on
> HeapTupleHeaderGetXmax(), this would break code that looks at
> xmin. Removing/renaming *GetXmin similarly sounds like a good idea to
> make the breakage explicit.

I thought about that, but there are enough callers that don't care
that it seemed like the wrong choice to me.

>> > I am all for adding a comment why this is safe though. We thought about
>> > it for some while before we were convinced...
>>
>> I'm fine with that, but the logic is present in multiple places, and I
>> did not want to comment them all identically. If there's a central
>> place in which a comment would be appropriate, let's add it there; or
>> IOW, what do you suggest in detail?
>
> That's a good point. Generally lots of this is underdocumented/commented
> and can only learned by spending a year or so in the postgres code. I'd
> say rename README.HOT to README and add a section there and reference it
> from those two places? It already has a mostly general explanation of
> concurrent index builds. Don't have a better idea.

Anyone else have a suggestion?

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