On Tue, Oct 29, 2013 at 11:37 AM, Andres Freund <andres@2ndquadrant.com> wrote:
> On 2013-10-29 11:29:24 -0400, Robert Haas wrote:
>> On Tue, Oct 29, 2013 at 10:32 AM, Andres Freund <andres@2ndquadrant.com> wrote:
>> > On 2013-10-25 09:26:29 -0400, Robert Haas wrote:
>> >> > In any case, it's very far from obvious to me that CLUSTER ought
>> >> > to throw away information by default, which is what you're proposing.
>> >>
>> >> I find it odd to referring to this as throwing away information. I
>> >> know that you have a general concern about throwing away XIDs that are
>> >> still needed for forensic purposes, but that is clearly the ONLY
>> >> purpose that those XIDs serve, and the I/O advantages of freezing by
>> >> default could be massive for many of our users. What's going to
>> >> happen in practice is that experienced users will simply recommend
>> >> CLUSTER FREEZE rather than plain CLUSTER, and you won't have the
>> >> forensic information *anyway*.
>> >
>> > I think we should just apply your "preserve forensic information when
>> > freezing" patch. Then we're good to go without big arguments ;)
>>
>> Well, I'm happy with that, too. But you wanted it significantly
>> reworked and I haven't had time to do that.
>
> I did? I only seem to remember suggesting to introduce
> HeapTupleHeaderGetRawXmin() and some bugfix around rewriteheap.c? I
> think the RawXmin() thing is a judgement call...
Well every place that currently gets the xmin will have to be changed
to get the raw-xmin instead, with the exception of hunks like this:
- targetxmin = HeapTupleHeaderGetXmin(tuple->t_data);
+ if (HeapTupleHeaderXminFrozen(tuple->t_data))
+ targetxmin = FrozenTransactionId;
+ else
+ targetxmin = HeapTupleHeaderGetXmin(tuple->t_data);
...which will instead need to be reverted. The rename is mostly
mechanical, but going through and looking for places where the
difference between Xmin() and RawXmin() means that other hunks can be
reverted is less so. I suppose it wouldn't take more than a few
hours; I've just been up to my ears in parallelism stuff.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
