Re: [HACKERS] CLUSTER FREEZE

2014-01-02 Thread Robert Haas
On Mon, Dec 23, 2013 at 6:53 AM, Andres Freund and...@2ndquadrant.com wrote:
 On 2013-12-22 20:45:02 -0500, Robert Haas wrote:
 I suspect we ought to extend this to rewriting variants of ALTER TABLE
 as well, but a little thought is needed there.  ATRewriteTables()
 appears to just call heap_insert() for each updated row, which if I'm
 not mistaken is an MVCC violation - offhand, it seems like a
 transaction with an older MVCC snapshot could access the table for
 this first time after the rewriter commits, and fail to see rows which
 would have appeared to be there before the rewrite. (I haven't
 actually tested this, so watch me be wrong.)  If we're OK with an MVCC
 violation here, we could just pass HEAP_INSERT_FROZEN and have a
 slightly different flavor of violation; if not, this needs some kind
 of more extensive surgery quite apart from what we do about freezing.

 Yes, rewriting ALTER TABLEs certainly aren't MVCC safe. I thought that
 was documented, but apparently not.
 I am not sure it can be fixed easily using the tricks CLUSTER plays,
 there might be nasty edge-cases because of the changes in the column
 definition. Certainly not a trivial project.

 I think we should leave ALTER TABLE as a completely separate project and
 just improve CLUSTER for now. The practical impact of rewriting ALTER
 TABLEs not freezing is far smaller, because they are very seldomly
 performed in bigger databases.

OK, I have committed my patch to make CLUSTER and VACUUM FULL freeze
aggressively, and have left ALTER TABLE alone for now.

It would be nice to get to the point where a database-wide VACUUM FULL
would serve to bump datfrozenxid, so as to avoid having to give this
sort of advice:

http://www.postgresql.org/message-id/CA+Tgmobth=aqkwmwtcsqlaenv59gt4g3oqpqs45cb+fvg9m...@mail.gmail.com

However, it doesn't, quite: a bare VACUUM FULL now bumps relfrozenxid
for every table in the database *except pg_class*.  It does call
vac_update_datfrozenxid() afterwards, but that only helps if pg_class
is not among the things holding back datfrozenxid.  I haven't dug into
this much yet, but I think it might be worth fixing.

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


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] CLUSTER FREEZE

2013-12-23 Thread Andres Freund
On 2013-12-22 20:45:02 -0500, Robert Haas wrote:
 I suspect we ought to extend this to rewriting variants of ALTER TABLE
 as well, but a little thought is needed there.  ATRewriteTables()
 appears to just call heap_insert() for each updated row, which if I'm
 not mistaken is an MVCC violation - offhand, it seems like a
 transaction with an older MVCC snapshot could access the table for
 this first time after the rewriter commits, and fail to see rows which
 would have appeared to be there before the rewrite. (I haven't
 actually tested this, so watch me be wrong.)  If we're OK with an MVCC
 violation here, we could just pass HEAP_INSERT_FROZEN and have a
 slightly different flavor of violation; if not, this needs some kind
 of more extensive surgery quite apart from what we do about freezing.

Yes, rewriting ALTER TABLEs certainly aren't MVCC safe. I thought that
was documented, but apparently not.
I am not sure it can be fixed easily using the tricks CLUSTER plays,
there might be nasty edge-cases because of the changes in the column
definition. Certainly not a trivial project.

I think we should leave ALTER TABLE as a completely separate project and
just improve CLUSTER for now. The practical impact of rewriting ALTER
TABLEs not freezing is far smaller, because they are very seldomly
performed in bigger databases.

Greetings,

Andres Freund

-- 
 Andres Freund http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training  Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] CLUSTER FREEZE

2013-12-22 Thread Robert Haas
On Sun, Dec 8, 2013 at 10:51 AM, Peter Eisentraut pete...@gmx.net wrote:
 On Tue, 2013-11-19 at 18:24 +0100, Andres Freund wrote:
 On 2013-11-19 12:23:30 -0500, Robert Haas wrote:
  On Mon, Nov 18, 2013 at 11:45 AM, Andres Freund and...@2ndquadrant.com 
  wrote:
   Yes, we probably should make a decision, unless Robert's idea can be
   implemented.  We have to balance the ease of debugging MVCC failures
   with the interface we give to the user community.
  
   Imo that patch really doesn't need too much further work.
 
  Would you care to submit a version you're happy with?

 I'll give it a spin sometime this week.

 I'm setting the CLUSTER FREEZE patch as returned with feedback, until
 this other patch has been considered.

The other patch in question is now committed.  Here's a patch for
this, which does somewhat more extensive surgery than previously
proposed (actually, I wrote it from scratch, before looking at the
prior submission).  It's basically the same idea, though.  Note that
both versions of the patch affect not only CLUSTER, but also VACUUM
FULL.

I suspect we ought to extend this to rewriting variants of ALTER TABLE
as well, but a little thought is needed there.  ATRewriteTables()
appears to just call heap_insert() for each updated row, which if I'm
not mistaken is an MVCC violation - offhand, it seems like a
transaction with an older MVCC snapshot could access the table for
this first time after the rewriter commits, and fail to see rows which
would have appeared to be there before the rewrite. (I haven't
actually tested this, so watch me be wrong.)  If we're OK with an MVCC
violation here, we could just pass HEAP_INSERT_FROZEN and have a
slightly different flavor of violation; if not, this needs some kind
of more extensive surgery quite apart from what we do about freezing.

Review of the attached appreciated...

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
diff --git a/doc/src/sgml/ref/vacuum.sgml b/doc/src/sgml/ref/vacuum.sgml
index eb71581..1e98473 100644
--- a/doc/src/sgml/ref/vacuum.sgml
+++ b/doc/src/sgml/ref/vacuum.sgml
@@ -102,7 +102,9 @@ VACUUM [ FULL ] [ FREEZE ] [ VERBOSE ] ANALYZE [ replaceable class=PARAMETER
   Specifying literalFREEZE/literal is equivalent to performing
   commandVACUUM/command with the
   xref linkend=guc-vacuum-freeze-min-age parameter
-  set to zero.
+  set to zero.  Aggressive freezing is always performed when the
+  table is rewritten, so this option is redundant when literalFULL/
+  is specified.
  /para
 /listitem
/varlistentry
diff --git a/src/backend/access/heap/rewriteheap.c b/src/backend/access/heap/rewriteheap.c
index deec77d..634754c 100644
--- a/src/backend/access/heap/rewriteheap.c
+++ b/src/backend/access/heap/rewriteheap.c
@@ -345,7 +345,7 @@ rewrite_heap_tuple(RewriteState state,
 
 	/*
 	 * While we have our hands on the tuple, we may as well freeze any
-	 * very-old xmin or xmax, so that future VACUUM effort can be saved.
+	 * eligible xmin or xmax, so that future VACUUM effort can be saved.
 	 */
 	heap_freeze_tuple(new_tuple-t_data, state-rs_freeze_xid,
 	  state-rs_cutoff_multi);
diff --git a/src/backend/commands/cluster.c b/src/backend/commands/cluster.c
index 0b8ac8c..9f41278 100644
--- a/src/backend/commands/cluster.c
+++ b/src/backend/commands/cluster.c
@@ -64,12 +64,10 @@ typedef struct
 } RelToCluster;
 
 
-static void rebuild_relation(Relation OldHeap, Oid indexOid,
- int freeze_min_age, int freeze_table_age, bool verbose);
+static void rebuild_relation(Relation OldHeap, Oid indexOid, bool verbose);
 static void copy_heap_data(Oid OIDNewHeap, Oid OIDOldHeap, Oid OIDOldIndex,
-			   int freeze_min_age, int freeze_table_age, bool verbose,
-			   bool *pSwapToastByContent, TransactionId *pFreezeXid,
-			   MultiXactId *pCutoffMulti);
+			   bool verbose, bool *pSwapToastByContent,
+			   TransactionId *pFreezeXid, MultiXactId *pCutoffMulti);
 static List *get_tables_to_cluster(MemoryContext cluster_context);
 static void reform_and_rewrite_tuple(HeapTuple tuple,
 		 TupleDesc oldTupDesc, TupleDesc newTupDesc,
@@ -176,11 +174,8 @@ cluster(ClusterStmt *stmt, bool isTopLevel)
 		/* close relation, keep lock till commit */
 		heap_close(rel, NoLock);
 
-		/*
-		 * Do the job.  We use a -1 freeze_min_age to avoid having CLUSTER
-		 * freeze tuples earlier than a plain VACUUM would.
-		 */
-		cluster_rel(tableOid, indexOid, false, stmt-verbose, -1, -1);
+		/* Do the job. */
+		cluster_rel(tableOid, indexOid, false, stmt-verbose);
 	}
 	else
 	{
@@ -229,9 +224,8 @@ cluster(ClusterStmt *stmt, bool isTopLevel)
 			StartTransactionCommand();
 			/* functions in indexes may want a snapshot set */
 			PushActiveSnapshot(GetTransactionSnapshot());
-			/* Do the job.  As above, use a -1 freeze_min_age. */
-			cluster_rel(rvtc-tableOid, rvtc-indexOid, true, stmt-verbose,
-		-1, -1);
+			/* Do the job. */
+			cluster_rel(rvtc-tableOid, 

Re: [HACKERS] CLUSTER FREEZE

2013-12-08 Thread Peter Eisentraut
On Tue, 2013-11-19 at 18:24 +0100, Andres Freund wrote:
 On 2013-11-19 12:23:30 -0500, Robert Haas wrote:
  On Mon, Nov 18, 2013 at 11:45 AM, Andres Freund and...@2ndquadrant.com 
  wrote:
   Yes, we probably should make a decision, unless Robert's idea can be
   implemented.  We have to balance the ease of debugging MVCC failures
   with the interface we give to the user community.
  
   Imo that patch really doesn't need too much further work.
  
  Would you care to submit a version you're happy with?
 
 I'll give it a spin sometime this week.

I'm setting the CLUSTER FREEZE patch as returned with feedback, until
this other patch has been considered.



-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] CLUSTER FREEZE

2013-11-20 Thread David Rowley
On Tue, Nov 19, 2013 at 11:35 PM, David Rowley dgrowle...@gmail.com wrote:

 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.


Hi Thomas,

I'm just going to mark the patch as waiting for author for now until you
get the chance to add some changes to the documents.

Everything else looks ok.

Regards

David Rowley


 Regards

 David Rowley




 Thanks
 Thomas Munro


 --
 Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
 To make changes to your subscription:
 http://www.postgresql.org/mailpref/pgsql-hackers





Re: [HACKERS] CLUSTER FREEZE

2013-11-19 Thread David Rowley
On Sat, Oct 26, 2013 at 11:19 AM, Thomas Munro mu...@ip9.org wrote:

 On 25 October 2013 01:17, Josh Berkus j...@agliodbs.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@postgresql.org)
 To make changes to your subscription:
 http://www.postgresql.org/mailpref/pgsql-hackers




cluster_freeze_always_with_docs.patch
Description: Binary data

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] CLUSTER FREEZE

2013-11-19 Thread Robert Haas
On Mon, Nov 18, 2013 at 11:45 AM, Andres Freund and...@2ndquadrant.com wrote:
 Yes, we probably should make a decision, unless Robert's idea can be
 implemented.  We have to balance the ease of debugging MVCC failures
 with the interface we give to the user community.

 Imo that patch really doesn't need too much further work.

Would you care to submit a version you're happy with?

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


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] CLUSTER FREEZE

2013-11-19 Thread Andres Freund
On 2013-11-19 12:23:30 -0500, Robert Haas wrote:
 On Mon, Nov 18, 2013 at 11:45 AM, Andres Freund and...@2ndquadrant.com 
 wrote:
  Yes, we probably should make a decision, unless Robert's idea can be
  implemented.  We have to balance the ease of debugging MVCC failures
  with the interface we give to the user community.
 
  Imo that patch really doesn't need too much further work.
 
 Would you care to submit a version you're happy with?

I'll give it a spin sometime this week.

Greetings,

Andres Freund

-- 
 Andres Freund http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training  Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] CLUSTER FREEZE

2013-11-18 Thread David Rowley
On Wed, Oct 30, 2013 at 3:32 AM, Andres Freund and...@2ndquadrant.comwrote:

 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 ;)


Ok, here's a recap on the thread as I see it now.

1. Thomas wrote the patch with the idea that FREEZE could be an option for
cluster to freeze the tuples during the cluster operation, which would save
a vacuum freeze somewhere down the line. Sounds like a good idea.
2. Robert introduced the idea that this perhaps could be the default option
for cluster.
3. Tom highlighted that making freeze the default would wipe out xmin
values so they wouldn't be available to any forensics which might to take
place in the event of a disaster.
4. Andres mentioned that we might want to wait for a patch which Robert has
been working on which, currently I don't know much about but it sounds like
it freezes without setting xmin to frozenXid?
5. Robert stated that he's not had much time to work on this patch due to
having other commitments.

In the meantime Thomas sent in a patch which gets rid of the FREEZE option
from cluster and makes it the default.

So now I'm wondering what the next move should be for this patch?

a. Are we waiting on Robert's patch to be commited before we can apply
Thomas's cluster with freeze as default?
b. Are we waiting on me reviewing one or both of the patches? Which one?

I think probably it sounds safer not to make freeze the default, but then
if we release 9.4 with CLUSTER FREEZE then in 9.5/10 decide it should be
the default then we then have a redundant syntax to support for ever and
ever.

Decision time?

Regards

David Rowley



 Greetings,

 Andres Freund

 --
  Andres Freund http://www.2ndQuadrant.com/
  PostgreSQL Development, 24x7 Support, Training  Services


 --
 Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
 To make changes to your subscription:
 http://www.postgresql.org/mailpref/pgsql-hackers



Re: [HACKERS] CLUSTER FREEZE

2013-11-18 Thread Bruce Momjian
On Mon, Nov 18, 2013 at 09:22:58PM +1300, David Rowley wrote:
 So now I'm wondering what the next move should be for this patch?
 
 a. Are we waiting on Robert's patch to be committed before we can apply 
 Thomas's
 cluster with freeze as default?
 b. Are we waiting on me reviewing one or both of the patches? Which one?
 
 I think probably it sounds safer not to make freeze the default, but then if 
 we
 release 9.4 with CLUSTER FREEZE then in 9.5/10 decide it should be the default
 then we then have a redundant syntax to support for ever and ever.
 
 Decision time?

Yes, we probably should make a decision, unless Robert's idea can be
implemented.  We have to balance the ease of debugging MVCC failures
with the interface we give to the user community.

From my perspective, I don't see how we can justify the addition of a
FREEZE option to CLUSTER.  If we explain that you would always use
FREEZE unless you want to preserve MVCC information for later debugging,
users will reply with Huh, why would I want that?  MVCC debugging is
just too rare an event for them to understand its usefulness.

If we do add FREEZE, all we would be doing is delaying the time when all
CLUSTER operations will use FREEZE, and hence debugging will be just as
difficult.  My point is that will full knowledge, everyone would use
FREEZE unless they expect MVCC bugs, which is going to be an almost zero
population.

Many MVCC failures are reproducible, so if we provide a C define that
disables the freezing so MVCC information can be preserved, that might
be good enough.  Developers could enable the macro, and the macro could
be used in other places where MVCC information is lost.  

This will make some MVCC harder, and will perhaps allow some MVCC bugs
to exist longer.

I believe there are other cases where rows could be frozen but we have
avoided it for MVCC debugging reasons.  Another idea would be the
addition of a GUC that can disable optimistic freezing. This could be
enabled by sites that suspect MVCC bugs.

-- 
  Bruce Momjian  br...@momjian.ushttp://momjian.us
  EnterpriseDB http://enterprisedb.com

  + Everyone has their own god. +


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] CLUSTER FREEZE

2013-11-18 Thread Andres Freund
On 2013-11-18 11:39:44 -0500, Bruce Momjian wrote:
 On Mon, Nov 18, 2013 at 09:22:58PM +1300, David Rowley wrote:
  So now I'm wondering what the next move should be for this patch?
  
  a. Are we waiting on Robert's patch to be committed before we can apply 
  Thomas's
  cluster with freeze as default?
  b. Are we waiting on me reviewing one or both of the patches? Which one?
  
  I think probably it sounds safer not to make freeze the default, but then 
  if we
  release 9.4 with CLUSTER FREEZE then in 9.5/10 decide it should be the 
  default
  then we then have a redundant syntax to support for ever and ever.
  
  Decision time?
 
 Yes, we probably should make a decision, unless Robert's idea can be
 implemented.  We have to balance the ease of debugging MVCC failures
 with the interface we give to the user community.

Imo that patch really doesn't need too much further work.

 From my perspective, I don't see how we can justify the addition of a
 FREEZE option to CLUSTER.  If we explain that you would always use
 FREEZE unless you want to preserve MVCC information for later debugging,
 users will reply with Huh, why would I want that?

I tend to agree that we should work towards not needing that option.

 Many MVCC failures are reproducible, so if we provide a C define that
 disables the freezing so MVCC information can be preserved, that might
 be good enough.  Developers could enable the macro, and the macro could
 be used in other places where MVCC information is lost.

 This will make some MVCC harder, and will perhaps allow some MVCC bugs
 to exist longer.

 I believe there are other cases where rows could be frozen but we have
 avoided it for MVCC debugging reasons.  Another idea would be the
 addition of a GUC that can disable optimistic freezing. This could be
 enabled by sites that suspect MVCC bugs.

I think that'd be an enormous failure. You don't usually need to look at
those because there's a bug in postgres' mvcc logic but somewhere
else (application, other postgres code). And looking at the mvcc
information helps you to narrow it down, so you have a chance of
actually finding a reproducible bug.
Besides, in many of the !rewrite cases it's far from clear in which
cases it's a benefit to freeze earlier.

Greetings,

Andres Freund

-- 
 Andres Freund http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training  Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] CLUSTER FREEZE

2013-11-18 Thread Josh Berkus
On 11/18/2013 08:39 AM, Bruce Momjian wrote:
 If we do add FREEZE, all we would be doing is delaying the time when all
 CLUSTER operations will use FREEZE, and hence debugging will be just as
 difficult.  My point is that will full knowledge, everyone would use
 FREEZE unless they expect MVCC bugs, which is going to be an almost zero
 population.

+1

None of our users would willingly choose worse performance over the 0.1%
possibility of needing to analyze a transaction failure.

-- 
Josh Berkus
PostgreSQL Experts Inc.
http://pgexperts.com


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] CLUSTER FREEZE

2013-11-18 Thread Kevin Grittner
Josh Berkus j...@agliodbs.com wrote:
 On 11/18/2013 08:39 AM, Bruce Momjian wrote:
 If we do add FREEZE, all we would be doing is delaying the time
 when all CLUSTER operations will use FREEZE, and hence debugging
 will be just as difficult.  My point is that will full
 knowledge, everyone would use FREEZE unless they expect MVCC
 bugs, which is going to be an almost zero population.

 +1

+1

 None of our users would willingly choose worse performance over
 the 0.1% possibility of needing to analyze a transaction failure.

I assume that's intended to represent the lifetime probability that
a typical user would ever benefit, not per year or per transaction.
Even as a lifetime number, it seems high unless we're specifically
talking about hackers.

--
Kevin Grittner
EDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] CLUSTER FREEZE

2013-10-29 Thread Andres Freund
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 ;)

Greetings,

Andres Freund

-- 
 Andres Freund http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training  Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] CLUSTER FREEZE

2013-10-29 Thread Robert Haas
On Tue, Oct 29, 2013 at 10:32 AM, Andres Freund and...@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.

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


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] CLUSTER FREEZE

2013-10-29 Thread Andres Freund
On 2013-10-29 11:29:24 -0400, Robert Haas wrote:
 On Tue, Oct 29, 2013 at 10:32 AM, Andres Freund and...@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...

Greetings,

Andres Freund

-- 
 Andres Freund http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training  Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] CLUSTER FREEZE

2013-10-29 Thread Robert Haas
On Tue, Oct 29, 2013 at 11:37 AM, Andres Freund and...@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 and...@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


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] CLUSTER FREEZE

2013-10-27 Thread Craig Ringer
On 10/26/2013 01:20 AM, Josh Berkus wrote:
 On 10/24/2013 07:19 PM, Tom Lane 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.
 
 The problem here is that you're thinking of the 1/10 of 1% of our users
 who have a serious PostgreSQL failure and post something on the lists
 for help, for which XID forensic information is useful.  As opposed to
 the 99.9% of our users for whom deferred freezing is a performance
 burden.  While I realize that the 0.1% of users are more likely to have
 contact with you, personally, it's still bad policy for the project.

Strong +1 from me. Doing the performant, sensible, low-admin thing by
default is really important if you don't want a database that requires a
two year training course and a professional DBA to use. Some DB vendors
make that part of their business model, but personally at least that's
certainly not the direction I'd like to see Pg go in.

Autovacuum wrap-around prevention already gets rid of this info, it's
not like it's kept forever anyway.

Anything that makes the mechanics of bloat and vacuum less visible is a
big win as far as I'm concerned.


-- 
 Craig Ringer   http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training  Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] CLUSTER FREEZE

2013-10-25 Thread Andres Freund
On 2013-10-24 17:17:22 -0700, Josh Berkus wrote:
 On 10/24/2013 04:55 PM, Robert Haas wrote:
  On Thu, Oct 24, 2013 at 1:09 PM, Josh Berkus j...@agliodbs.com wrote:
  On 10/23/2013 09:58 PM, Amit Kapila wrote:
  I wonder why anyone would like to freeze during CLUSTER command when
  they already have separate way (VACUUM FREEZE) to achieve it, do you
  know or can think of any case where user wants to do it along with
  Cluster command?
 
  If I'm rewriting the table anyway, let's freeze it.
 
  Otherwise, you have to write the same pages twice, if both CLUSTER and
  FREEZE are required.
  
  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?

It makes content from the future appear when you start using the
relation in a query/session with an older snapshot. Currently CLUSTER is
safe against that.

Greetings,

Andres Freund

-- 
 Andres Freund http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training  Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] CLUSTER FREEZE

2013-10-25 Thread Robert Haas
On Fri, Oct 25, 2013 at 2:12 AM, Andres Freund and...@2ndquadrant.com wrote:
 On 2013-10-24 17:17:22 -0700, Josh Berkus wrote:
 On 10/24/2013 04:55 PM, Robert Haas wrote:
  On Thu, Oct 24, 2013 at 1:09 PM, Josh Berkus j...@agliodbs.com wrote:
  On 10/23/2013 09:58 PM, Amit Kapila wrote:
  I wonder why anyone would like to freeze during CLUSTER command when
  they already have separate way (VACUUM FREEZE) to achieve it, do you
  know or can think of any case where user wants to do it along with
  Cluster command?
 
  If I'm rewriting the table anyway, let's freeze it.
 
  Otherwise, you have to write the same pages twice, if both CLUSTER and
  FREEZE are required.
 
  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?

 It makes content from the future appear when you start using the
 relation in a query/session with an older snapshot. Currently CLUSTER is
 safe against that.

Eh, what?  We wouldn't freeze XIDs that don't precede RecentGlobalXmin.

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


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] CLUSTER FREEZE

2013-10-25 Thread Andres Freund
On 2013-10-25 09:13:20 -0400, Robert Haas wrote:
 On Fri, Oct 25, 2013 at 2:12 AM, Andres Freund and...@2ndquadrant.com wrote:
  On 2013-10-24 17:17:22 -0700, Josh Berkus wrote:
  On 10/24/2013 04:55 PM, Robert Haas wrote:
   On Thu, Oct 24, 2013 at 1:09 PM, Josh Berkus j...@agliodbs.com wrote:
   On 10/23/2013 09:58 PM, Amit Kapila wrote:
   I wonder why anyone would like to freeze during CLUSTER command when
   they already have separate way (VACUUM FREEZE) to achieve it, do you
   know or can think of any case where user wants to do it along with
   Cluster command?
  
   If I'm rewriting the table anyway, let's freeze it.
  
   Otherwise, you have to write the same pages twice, if both CLUSTER and
   FREEZE are required.
  
   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?
 
  It makes content from the future appear when you start using the
  relation in a query/session with an older snapshot. Currently CLUSTER is
  safe against that.
 
 Eh, what?  We wouldn't freeze XIDs that don't precede RecentGlobalXmin.

Ah sorry, I thought that'd be the plan, similar to COPY FREEZE. Maybe
because I've wished for it in the past ;)

In that case I agree it should be the default. There really isn't any
reason to not to immediately freeze tuples that can be frozen according
to the xmin horizon. We don't immediately do it during normal vacuums
because it would possibly cause superflous io - but that's not the case here.

Greetings,

Andres Freund

-- 
 Andres Freund http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training  Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] CLUSTER FREEZE

2013-10-25 Thread Robert Haas
On Thu, Oct 24, 2013 at 10:19 PM, Tom Lane t...@sss.pgh.pa.us wrote:
 Robert Haas robertmh...@gmail.com writes:
 I wonder if we should go so far as to make this the default behavior,
 instead of just making it an option.

 In that case you'd have to invent a NOFREEZE keyword, no?  Ick.

Only if we think anyone would ever NOT want to freeze.

 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*.

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


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] CLUSTER FREEZE

2013-10-25 Thread Josh Berkus
On 10/24/2013 07:19 PM, Tom Lane 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.

The problem here is that you're thinking of the 1/10 of 1% of our users
who have a serious PostgreSQL failure and post something on the lists
for help, for which XID forensic information is useful.  As opposed to
the 99.9% of our users for whom deferred freezing is a performance
burden.  While I realize that the 0.1% of users are more likely to have
contact with you, personally, it's still bad policy for the project.

-- 
Josh Berkus
PostgreSQL Experts Inc.
http://pgexperts.com


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] CLUSTER FREEZE

2013-10-25 Thread Thomas Munro
On 25 October 2013 01:17, Josh Berkus j...@agliodbs.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.

Thanks
Thomas Munro


cluster-freeze-always.patch
Description: Binary data

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] CLUSTER FREEZE

2013-10-24 Thread Thom Brown
On 24 October 2013 05:58, Amit Kapila amit.kapil...@gmail.com wrote:
 On Thu, Oct 24, 2013 at 4:58 AM, Thomas Munro mu...@ip9.org wrote:
 Hi
 I noticed that CLUSTER doesn't have a FREEZE option.  Here is a patch to add
 that, for consistency with VACUUM.  Is it useful?

 I wonder why anyone would like to freeze during CLUSTER command when
 they already have separate way (VACUUM FREEZE) to achieve it, do you
 know or can think of any case where user wants to do it along with
 Cluster command?

Well I can't speak for Thomas, but if you're doing such a heavy-duty
operation on a table that involves an exclusive lock, you may as well
freeze it.  And in the case of CLUSTER, I imagine that in most
instances you'd be confident the table isn't going to undergo much
change (or at least not quickly).  CLUSTER FREEZE would mean not
having to run a separate VACUUM FREEZE after, and on a 10GB table,
that's a big deal as it means not having to rescan the whole table
again to freeze every row.

Note that REFRESH MATERIALIZED VIEW freezes rows too as it's already
writing all the data from scratch again, and has an exclusive lock.
(this isn't the case with the CONCURRENTLY option obviously)

-- 
Thom


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] CLUSTER FREEZE

2013-10-24 Thread Andres Freund
Hi,

On 2013-10-24 00:28:44 +0100, Thomas Munro wrote:
 I noticed that CLUSTER doesn't have a FREEZE option.  Here is a patch to
 add that, for consistency with VACUUM.  Is it useful?

I think you'd need to prevent that form from working on system catalogs
- xmin has a meaning there sometimes (e.g. indcheckxmin) at least.

Greetings,

Andres Freund

-- 
 Andres Freund http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training  Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] CLUSTER FREEZE

2013-10-24 Thread k...@rice.edu
On Thu, Oct 24, 2013 at 10:28:43AM +0530, Amit Kapila wrote:
 On Thu, Oct 24, 2013 at 4:58 AM, Thomas Munro mu...@ip9.org wrote:
  Hi
  I noticed that CLUSTER doesn't have a FREEZE option.  Here is a patch to add
  that, for consistency with VACUUM.  Is it useful?
 
 I wonder why anyone would like to freeze during CLUSTER command when
 they already have separate way (VACUUM FREEZE) to achieve it, do you
 know or can think of any case where user wants to do it along with
 Cluster command?
 
 Anyway code side, I think you need to set both feeze_min_age as well
 as freeze_table_age, see VACUUM command in gram.y
 
 CLUSTER opt_freeze opt_verbose qualified_name cluster_index_specification
 
   {
   ClusterStmt *n = makeNode(ClusterStmt);
 - n-relation = $3;
 - n-indexname = $4;
 - n-verbose = $2;
 + n-relation = $4;
 + n-freeze_min_age = $2 ? 0 : -1;
 + n-indexname = $5;
 + n-verbose = $3;
 ..
 
 With Regards,
 Amit Kapila.
 EnterpriseDB: http://www.enterprisedb.com
 

Hi Amit,

If the FREEZE is part of the CLUSTER, you would only read/write the table
once. With a follow-up VACUUM FREEZE, you would re-read/write a second time.
I, for one, would appreciate being able to perform both in the same run. (+1)

Regards,
Ken


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] CLUSTER FREEZE

2013-10-24 Thread Josh Berkus
On 10/23/2013 09:58 PM, Amit Kapila wrote:
 I wonder why anyone would like to freeze during CLUSTER command when
 they already have separate way (VACUUM FREEZE) to achieve it, do you
 know or can think of any case where user wants to do it along with
 Cluster command?

If I'm rewriting the table anyway, let's freeze it.

Otherwise, you have to write the same pages twice, if both CLUSTER and
FREEZE are required.

-- 
Josh Berkus
PostgreSQL Experts Inc.
http://pgexperts.com


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] CLUSTER FREEZE

2013-10-24 Thread Thomas Munro
On 24 October 2013 05:58, Amit Kapila amit.kapil...@gmail.com wrote:

 On Thu, Oct 24, 2013 at 4:58 AM, Thomas Munro mu...@ip9.org wrote:
  Hi
  I noticed that CLUSTER doesn't have a FREEZE option.  Here is a patch to
 add
  that, for consistency with VACUUM.  Is it useful?

 I wonder why anyone would like to freeze during CLUSTER command when
 they already have separate way (VACUUM FREEZE) to achieve it, do you
 know or can think of any case where user wants to do it along with
 Cluster command?


As others have said, the goal is to freeze and cluster in a single step.
 You can already do that if you know how things work under the covers with:

SET vacuum_freeze_min_age = 0;
CLUSTER my_table;

This patch lets you say CLUSTER FREEZE instead.  It mirrors VACUUM, which
can freeze tuples based on the GUC and tuple age or the FREEZE keyword.


 Anyway code side, I think you need to set both feeze_min_age as well
 as freeze_table_age, see VACUUM command in gram.y


Ok, I attach a new version that is more like VACUUM in gram.y.  (Although
I'm not sure why it isn't a single boolean flag).

Thanks,
Thomas Munro


cluster-freeze-v2.patch
Description: Binary data

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] CLUSTER FREEZE

2013-10-24 Thread Robert Haas
On Thu, Oct 24, 2013 at 1:09 PM, Josh Berkus j...@agliodbs.com wrote:
 On 10/23/2013 09:58 PM, Amit Kapila wrote:
 I wonder why anyone would like to freeze during CLUSTER command when
 they already have separate way (VACUUM FREEZE) to achieve it, do you
 know or can think of any case where user wants to do it along with
 Cluster command?

 If I'm rewriting the table anyway, let's freeze it.

 Otherwise, you have to write the same pages twice, if both CLUSTER and
 FREEZE are required.

I wonder if we should go so far as to make this the default behavior,
instead of just making it an option.

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


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] CLUSTER FREEZE

2013-10-24 Thread Josh Berkus
On 10/24/2013 04:55 PM, Robert Haas wrote:
 On Thu, Oct 24, 2013 at 1:09 PM, Josh Berkus j...@agliodbs.com wrote:
 On 10/23/2013 09:58 PM, Amit Kapila wrote:
 I wonder why anyone would like to freeze during CLUSTER command when
 they already have separate way (VACUUM FREEZE) to achieve it, do you
 know or can think of any case where user wants to do it along with
 Cluster command?

 If I'm rewriting the table anyway, let's freeze it.

 Otherwise, you have to write the same pages twice, if both CLUSTER and
 FREEZE are required.
 
 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?

-- 
Josh Berkus
PostgreSQL Experts Inc.
http://pgexperts.com


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] CLUSTER FREEZE

2013-10-24 Thread Tom Lane
Robert Haas robertmh...@gmail.com writes:
 I wonder if we should go so far as to make this the default behavior,
 instead of just making it an option.

In that case you'd have to invent a NOFREEZE keyword, no?  Ick.

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.

regards, tom lane


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] CLUSTER FREEZE

2013-10-24 Thread Amit Kapila
On Thu, Oct 24, 2013 at 10:39 PM, Josh Berkus j...@agliodbs.com wrote:
 On 10/23/2013 09:58 PM, Amit Kapila wrote:
 I wonder why anyone would like to freeze during CLUSTER command when
 they already have separate way (VACUUM FREEZE) to achieve it, do you
 know or can think of any case where user wants to do it along with
 Cluster command?

 If I'm rewriting the table anyway, let's freeze it.

   So do you think that other places where we are rewriting the table
with exclusive lock, we should have such mechanism or option and even
if it is not there, it is kind of Todo item and tomorrow someone can
write a patch to improve that situation.

 Otherwise, you have to write the same pages twice, if both CLUSTER and
 FREEZE are required.

Yes, this is completely right and I understand this point, but the
question I had in my mind before writing my last mail was that whether
any or all places having this concept deserves to have an option like
FREEZE?

With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] CLUSTER FREEZE

2013-10-24 Thread Amit Kapila
On Fri, Oct 25, 2013 at 4:29 AM, Thomas Munro mu...@ip9.org wrote:
 On 24 October 2013 05:58, Amit Kapila amit.kapil...@gmail.com wrote:

 On Thu, Oct 24, 2013 at 4:58 AM, Thomas Munro mu...@ip9.org wrote:
  Hi
  I noticed that CLUSTER doesn't have a FREEZE option.  Here is a patch to
  add
  that, for consistency with VACUUM.  Is it useful?

 I wonder why anyone would like to freeze during CLUSTER command when
 they already have separate way (VACUUM FREEZE) to achieve it, do you
 know or can think of any case where user wants to do it along with
 Cluster command?


 As others have said, the goal is to freeze and cluster in a single step.
 You can already do that if you know how things work under the covers with:

 SET vacuum_freeze_min_age = 0;
 CLUSTER my_table;

   True, but in that case why don't we just mention above in the
documentation of CLUSTER command, so that if user wants to freeze
along with Cluster, he can use above way to Cluster.
Some of the reason's, I could think of adding FREEZE as an option are:
a. it's more explicit and easy to use for user.
b. if by chance underlying mechanism changes (which is less likely)
then above way of doing Cluster might not result into freeze.

 This patch lets you say CLUSTER FREEZE instead.  It mirrors VACUUM, which
 can freeze tuples based on the GUC and tuple age or the FREEZE keyword.


 Anyway code side, I think you need to set both feeze_min_age as well
 as freeze_table_age, see VACUUM command in gram.y


 Ok, I attach a new version that is more like VACUUM in gram.y.  (Although
 I'm not sure why it isn't a single boolean flag).
The reason of separate flags is that both are used to decide different things,
freeze_min_age - this is used to decide the cutoff_xid, based on which
FrozenTransactionId will be placed on tuple.
freeze_table_age - used do decide, whether to scan all pages of a
relation in Vacuum and in Cluster command it is ignored as it needs to
scan all pages anyway.

With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


[HACKERS] CLUSTER FREEZE

2013-10-23 Thread Thomas Munro
Hi
I noticed that CLUSTER doesn't have a FREEZE option.  Here is a patch to
add that, for consistency with VACUUM.  Is it useful?
Thanks
Thomas Munro


cluster-freeze.patch
Description: Binary data

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] CLUSTER FREEZE

2013-10-23 Thread Amit Kapila
On Thu, Oct 24, 2013 at 4:58 AM, Thomas Munro mu...@ip9.org wrote:
 Hi
 I noticed that CLUSTER doesn't have a FREEZE option.  Here is a patch to add
 that, for consistency with VACUUM.  Is it useful?

I wonder why anyone would like to freeze during CLUSTER command when
they already have separate way (VACUUM FREEZE) to achieve it, do you
know or can think of any case where user wants to do it along with
Cluster command?

Anyway code side, I think you need to set both feeze_min_age as well
as freeze_table_age, see VACUUM command in gram.y

CLUSTER opt_freeze opt_verbose qualified_name cluster_index_specification

  {
  ClusterStmt *n = makeNode(ClusterStmt);
- n-relation = $3;
- n-indexname = $4;
- n-verbose = $2;
+ n-relation = $4;
+ n-freeze_min_age = $2 ? 0 : -1;
+ n-indexname = $5;
+ n-verbose = $3;
..

With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers