Re: preserving forensic information when we freeze

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.

> 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?

> Hm. As previously said, I am less than convinced of those adhoc
> mechanisms and I think this should get properly integrated into the
> normal cache invalidation mechanisms.
> But: I think this is safe since we compare the stored/cached xmin/tid
> with one gotten from the SearchSysCache just before which will point to
> the correct location as of the last AcceptInvalidationMessages(). I
> can't think of a way were this would allow the case you describe.

I don't understand why it can't. AcceptInvalidationMessages()
guarantees that we're looking at the latest version of the catalog,
but it doesn't say anything about whether the latest catalog state
happens to look like any earlier catalog state.

> I don't think this is especially problematic though. If you do a tidscan
> starting from a tid that is so old that it can be removed: you're doing
> it wrong. The tid could have been reused for something else anyway. I
> think the ctid chaining is only meaningful if the tuple got updated very
> recently, i.e. you hold a snapshot that prevents the removal of the
> root tuple's snapshot.

That logic seems sound to me.

> Nice work!

Thanks!

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