Re: CLUSTER FREEZE

From: David Rowley <dgrowleyml(at)gmail(dot)com>
To: Thomas Munro <munro(at)ip9(dot)org>
Cc: Josh Berkus <josh(at)agliodbs(dot)com>, Robert Haas <robertmhaas(at)gmail(dot)com>, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>, "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: CLUSTER FREEZE
Date: 2013-11-19 10:35:55
Message-ID: CAApHDvrNLbm3PVqqj6tG73h1WxR_41C3ZNQCPACJxOemDEe=Hg@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Sat, Oct 26, 2013 at 11:19 AM, Thomas Munro <munro(at)ip9(dot)org> wrote:

> On 25 October 2013 01:17, Josh Berkus <josh(at)agliodbs(dot)com> wrote:
>
>> On 10/24/2013 04:55 PM, Robert Haas wrote:
>> > I wonder if we should go so far as to make this the default behavior,
>> > instead of just making it an option.
>>
>> +1 from me. Can you think of a reason you *wouldn't* want to freeze?
>>
>
> Ok, I attach an alternative patch that makes CLUSTER *always* freeze,
> without any option (but doesn't affect VACUUM FULL in the same way). I will
> post both alternatives to the commitfest app since there seems to be some
> disagreement about whether tuple freezing should be an optional.
>
>
It seems that most people to voice their opinion are leaning towards this
being the more desired behaviour rather than adding the FREEZE as an
option, so I reviewed this patch tonight.

I followed the code around and checked that we do still need the freeze age
parameters in cluster_rel and we do because vacuum full uses the
cluster_rel code and it will only perform a freeze if a user does VACUUM
FULL FREEZE.

I have mixed feelings about updating the comment before the call
to vacuum_set_xid_limits. It looks like the previous comment probably had
not been looked at since vacuum full started using cluster_rel, so perhaps
removing that was good, but on the other hand maybe it should be mentioning
vacuum full and vacuum full freeze? Though I'm probably leaning more
towards what you've changed it to as previously the comment was being a bit
too clever and assuming things about the calling code which turned out bad
as it seemed out of date and lacked knowledge of vacuum full using it.

I think that the patch should include some sort of notes in the documents
to say that cluster performs freezing of tuples. I've attached a patch
which adds something there, but I'm not 100% sure it is the right thing.
Perhaps it should just read:

Cluster also performs aggressive "freezing" of tuples similar to VACUUM
FULL FREEZE.

Although it's not exactly the same as you can perform a vacuum full freeze
on a relation which does not have the clustered index set.

I'll delay a bit to see if anyone else has any comments about what the docs
should read, but I think it is pretty much ready for a commiter's eyes.

Regards

David Rowley

> Thanks
> Thomas Munro
>
>
> --
> Sent via pgsql-hackers mailing list (pgsql-hackers(at)postgresql(dot)org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgsql-hackers
>
>

Attachment Content-Type Size
cluster_freeze_always_with_docs.patch application/octet-stream 1.8 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Amit Langote 2013-11-19 10:45:12 Re: Call flow of btinsert(PG_FUNCTION_ARGS)
Previous Message Rajeev rastogi 2013-11-19 10:35:09 Re: COPY table FROM STDIN doesn't show count tag