On Sat, Mar 3, 2012 at 14:53, Simon Riggs <simon@2ndquadrant.com> wrote:
> Thanks Noah for drawing attention to this thread. I hadn't been
> watching. As you say, this work would allow me to freeze rows at load
> time and avoid the overhead of hint bit setting, which avoids
> performance issues from hint bit setting in checksum patch.
>
> I've reviewed this patch and it seems OK to me. Good work Marti.
Thanks! This approach wasn't universally liked, but if it gets us
tangible benefits (COPY with frozenxid) then I guess it's a reason to
reconsider.
> v2 patch attached, updated to latest HEAD. Patch adds
> * relvalidxid is reset by VACUUM/ANALYZE to avoid wraparound failure
Personally I'd rather keep this out of ANALYZE -- since its purpose is
to collect stats; VACUUM is responsible for correctness (xid
wraparound etc). But I don't feel strongly about this.
A more important consideration is how this interacts with hot standby.
Currently you compare OldestXmin to relvalidxmin to decide when to
reset it. But the standby's OldestXmin may be older than the master's.
(If VACUUM removes rows then this causes a recovery conflict, but
AFAICT that won't happen if only relvalidxmin changes)
It might be more robust to wait until relfrozenxid exceeds
relvalidxmin -- by then, recovery conflict mechanisms will have taken
care of killing all older snapshots, or am I mistaken?
And a few typos the code...
+ gettext_noop("When enabled viewing a truncated or newly created table "
+ "will throw a serialization error to prevent MVCC "
+ "discrepancies that were possible priot to 9.2.")
"prior" not "priot"
+ * Reset relvalidxmin if its old enough
Should be "it's" in this context.
Regards,
Marti