thread-index: Acd95kg4RROaEDIYSe+oDxgeTx2SGA==
Content-Class: urn:content-classes:message
Importance: normal
Subject: Re: [HACKERS] Full page writes improvement, code update
From: "Simon Riggs" <simon@2ndquadrant.com>
To: "Tom Lane" <tgl@sss.pgh.pa.us>
Cc: "Zeugswetter Andreas ADI SD" <ZeugswetterA@spardat.at>,
	"Koichi Suzuki" <suzuki.koichi@oss.ntt.co.jp>,
	"Hannu Krosing" <hannu@skype.net>,
	"Joshua D. Drake" <jd@commandprompt.com>, <josh@agliodbs.com>,
	<pgsql-hackers@postgresql.org>, <pgsql-patches@postgresql.org>
In-Reply-To: <19755.1176479261@sss.pgh.pa.us>
References: <460886E8.6040907@oss.ntt.co.jp> <4609CAC3.7060008@oss.ntt.co.jp>
	<1175072848.4386.323.camel@silverbirch.site>
	<200703291145.28574.josh@agliodbs.com>
	<1175200117.4386.616.camel@silverbirch.site>
	<460C611E.6090800@oss.ntt.co.jp> <27693.1176226131@sss.pgh.pa.us>
	<461C3557.6040201@oss.ntt.co.jp> <461C3732.20006@commandprompt.com>
	<1176269612.3962.11.camel@localhost.localdomain>
	<461D9090.60808@oss.ntt.co.jp>
	<E1539E0ED7043848906A8FF995BDA57901E7BD64@m0143.s-mxs.net>
	<461ECA4E.9010101@oss.ntt.co.jp>
	<E1539E0ED7043848906A8FF995BDA57901E7BEBE@m0143.s-mxs.net>
	<19002.1176475004@sss.pgh.pa.us>
	<1176478259.3635.176.camel@silverbirch.site>
	<19755.1176479261@sss.pgh.pa.us>
Content-Type: text/plain;
	charset="iso-8859-1"
Date: Fri, 13 Apr 2007 17:06:08 +0100
Message-ID: <1176480368.3635.199.camel@silverbirch.site>
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit

On Fri, 2007-04-13 at 11:47 -0400, Tom Lane wrote:
> "Simon Riggs" <simon@2ndquadrant.com> writes:
> > On Fri, 2007-04-13 at 10:36 -0400, Tom Lane wrote:
> >> That's what bothers me about this patch, too.  It will be increasing
> >> the cost of writing WAL (more data -> more CRC computation and more
> >> I/O, not to mention more contention for the WAL locks) which translates
> >> directly to a server slowdown.
> 
> > I don't really understand this concern.
> 
> The real objection is that a patch that's alleged to make WAL smaller
> actually does the exact opposite.  Now maybe you can buy that back
> downstream of the archiver --- after yet more added-on processing ---
> but it still seems that there's a fundamental misdesign here.
> 
> > Koichi-san has included a parameter setting that would prevent any
> > change at all in the way WAL is written.
> 
> It bothers me that we'd need to have such a switch.  That's just another
> way to shoot yourself in the foot, either by not enabling it (in which
> case applying pg_compresslog as it stands would actively break your
> WAL), or by enabling it when you weren't actually going to use
> pg_compresslog (because you misunderstood the documentation to imply
> that it'd make your WAL smaller by itself).  What I want to see is a
> patch that doesn't bloat WAL at all and therefore doesn't need a switch.
> I think Andreas is correct to complain that it should be done that way.

I agree with everything you say because we already had *exactly* this
discussion when the patch was already submitted, with me saying
everything you just said.

After a few things have been renamed to show their correct function and
impact, I am now comfortable with this patch.

Writing lots of additional code simply to remove a parameter that
*might* be mis-interpreted doesn't sound useful to me, especially when
bugs may leak in that way. My take is that this is simple and useful
*and* we have it now; other ways don't yet exist, nor will they in time
for 8.3.

-- 
  Simon Riggs             
  EnterpriseDB   http://www.enterprisedb.com