Thread: Fast CLUSTER
I've used the heap_sync() API call to improve performance of CLUSTER by avoiding WAL when archive_command is not set. I've refactored the heap_sync() call very slightly, to hide some of the complexity that was previously exposed to COPY. It now syncs a TOAST relation automatically if one exists for the table. That also fixes a latent bug in CREATE TABLE AS SELECT which would have meant that the TOAST table wasn't properly synced (my bug), effectively caused by the original API design. Patch applies cleanly, passes make check. No user interface changes. Performance docs updated also to reflect this change and slightly improve directly related wording. -- Simon Riggs EnterpriseDB http://www.enterprisedb.com
Attachment
Simon Riggs wrote:
>
> I've used the heap_sync() API call to improve performance of CLUSTER by
> avoiding WAL when archive_command is not set.
Cool. I noticed that the SGML seems broken here:
> --- 908,925 ----
> will perform more slowly when <varname>archive_command</varname>
> is set, as a result of their needing to write large amounts of WAL.
> This applies to the following commands:
> ! <itemizedlist>
> ! <listitem><para><command>CREATE TABLE AS SELECT</command></para></listitem>
> ! <listitem><para><command>CREATE INDEX</command></para></listitem>
> ! <listitem><para><command>ALTER TABLE SET TABLESPACE</command></para></listitem>
> ! <listitem><para><command>CLUSTER</command></para></listitem>
> ! <listitem><para><command>COPY</command>, when it is executed after one of
> ! these commands, yet in the same transaction:
> ! <itemizedlist>
> ! <listitem><para><command>CREATE TABLE</command></para></listitem>
> ! <listitem><para><command>TRUNCATE</command></para></listitem>
> ! </itemizedlist>
> ! </itemizedlist>
> </sect2>
You need to close the <listitem> and <para> opened in the COPY mention.
> +
> + static void
> + heap_sync_relation(Relation rel)
> + {
> + if (!rel->rd_istemp)
No comment in this function?
--
Alvaro Herrera http://www.CommandPrompt.com/
PostgreSQL Replication, Consulting, Custom Development, 24x7 support
On Tue, 2007-02-20 at 14:38 -0300, Alvaro Herrera wrote:
> Cool. I noticed that the SGML seems broken here:
Corrected.
> You need to close the <listitem> and <para> opened in the COPY mention.
>
> > +
> > + static void
> > + heap_sync_relation(Relation rel)
> > + {
> > + if (!rel->rd_istemp)
>
> No comment in this function?
I've added more comments as you suggest.
Thanks for the review.
--
Simon Riggs
EnterpriseDB http://www.enterprisedb.com
Attachment
On Thu, 2007-02-22 at 22:49 +0000, Simon Riggs wrote:
> On Tue, 2007-02-20 at 14:38 -0300, Alvaro Herrera wrote:
>
> > Cool. I noticed that the SGML seems broken here:
>
> Corrected.
>
> > You need to close the <listitem> and <para> opened in the COPY mention.
> >
> > > +
> > > + static void
> > > + heap_sync_relation(Relation rel)
> > > + {
> > > + if (!rel->rd_istemp)
> >
> > No comment in this function?
>
> I've added more comments as you suggest.
>
> Thanks for the review.
Could we add this to the unapplied patches queue? It seems to have been
missed off the list. Thanks.
There is a probable conflict with Heikki's recent CLUSTER patch, so I'm
happy to re-write this patch after that has been applied. So its OK to
stick it at the bottom of the queue.
--
Simon Riggs
EnterpriseDB http://www.enterprisedb.com
Simon Riggs wrote:
> On Thu, 2007-02-22 at 22:49 +0000, Simon Riggs wrote:
> > On Tue, 2007-02-20 at 14:38 -0300, Alvaro Herrera wrote:
> >
> > > Cool. I noticed that the SGML seems broken here:
> >
> > Corrected.
> >
> > > You need to close the <listitem> and <para> opened in the COPY mention.
> > >
> > > > +
> > > > + static void
> > > > + heap_sync_relation(Relation rel)
> > > > + {
> > > > + if (!rel->rd_istemp)
> > >
> > > No comment in this function?
> >
> > I've added more comments as you suggest.
> >
> > Thanks for the review.
>
> Could we add this to the unapplied patches queue? It seems to have been
> missed off the list. Thanks.
Done.
--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://www.enterprisedb.com
+ If your life is a hard drive, Christ can be your backup. +
Your patch has been added to the PostgreSQL unapplied patches list at:
http://momjian.postgresql.org/cgi-bin/pgpatches
It will be applied as soon as one of the PostgreSQL committers reviews
and approves it.
---------------------------------------------------------------------------
Simon Riggs wrote:
> On Tue, 2007-02-20 at 14:38 -0300, Alvaro Herrera wrote:
>
> > Cool. I noticed that the SGML seems broken here:
>
> Corrected.
>
> > You need to close the <listitem> and <para> opened in the COPY mention.
> >
> > > +
> > > + static void
> > > + heap_sync_relation(Relation rel)
> > > + {
> > > + if (!rel->rd_istemp)
> >
> > No comment in this function?
>
> I've added more comments as you suggest.
>
> Thanks for the review.
>
> --
> Simon Riggs
> EnterpriseDB http://www.enterprisedb.com
>
[ Attachment, skipping... ]
>
> ---------------------------(end of broadcast)---------------------------
> TIP 5: don't forget to increase your free space map settings
--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://www.enterprisedb.com
+ If your life is a hard drive, Christ can be your backup. +
On Tue, 2007-03-27 at 10:28 -0400, Bruce Momjian wrote: > Simon Riggs wrote: > > On Thu, 2007-02-22 at 22:49 +0000, Simon Riggs wrote: > > Could we add this to the unapplied patches queue? It seems to have been > > missed off the list. Thanks. > > Done. Many thanks -- Simon Riggs EnterpriseDB http://www.enterprisedb.com
"Simon Riggs" <simon@2ndquadrant.com> writes:
> [ make CLUSTER skip WAL when possible ]
Applied with some editorialization.
regards, tom lane