Date: Tue, 19 Sep 2017 22:58:59 -0700
From: Andres Freund <andres@anarazel.de>
To: Michael Paquier <michael.paquier@gmail.com>
Cc: PostgreSQL mailing lists <pgsql-hackers@postgresql.org>
Subject: Re: [COMMITTERS] pgsql: Make WAL segment size configurable
 at initdb time.
Message-ID: <20170920055859.ymb3ls3cwsuzh5x6@alap3.anarazel.de>
References: <E1duXBW-0003QB-4d@gemulon.postgresql.org>
 <CAB7nPqTfaKAYZ4wuUM-W8kc4VnXrxX1=5-a9i==VoUPTMFpsgg@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: 
 <CAB7nPqTfaKAYZ4wuUM-W8kc4VnXrxX1=5-a9i==VoUPTMFpsgg@mail.gmail.com>
Precedence: bulk
Sender: pgsql-hackers-owner@postgresql.org

On 2017-09-20 14:51:45 +0900, Michael Paquier wrote:
> On Wed, Sep 20, 2017 at 2:04 PM, Andres Freund <andres@anarazel.de> wrote:
> > Make WAL segment size configurable at initdb time.
> >
> > For performance reasons a larger segment size than the default 16MB
> > can be useful. A larger segment size has two main benefits: Firstly,
> > in setups using archiving, it makes it easier to write scripts that
> > can keep up with higher amounts of WAL, secondly, the WAL has to be
> > written and synced to disk less frequently.
> >
> > But at the same time large segment size are disadvantageous for
> > smaller databases. So far the segment size had to be configured at
> > compile time, often making it unrealistic to choose one fitting to a
> > particularly load. Therefore change it to a initdb time setting.
> >
> > This includes a breaking changes to the xlogreader.h API, which now
> > requires the current segment size to be configured.  For that and
> > similar reasons a number of binaries had to be taught how to recognize
> > the current segment size.
> 
> I have been reading through this commit, and there is one change in
> streamutil.c which is really unwelcome:
> +   /* for previous versions set the default xlog seg size */
> +   if (PQserverVersion(conn) < MINIMUM_VERSION_FOR_SHOW_CMD)
> +   {
> +       WalSegSz = DEFAULT_XLOG_SEG_SIZE;
> +       return true;
> +   }
> Because of this pg_receivewal will generate silently incorrect WAL
> segment data if connecting with builds of PostgreSQL 10 or older
> versions which do not have the default segment size of 16MB set. If
> this patch justifies such breakages, I think that this should just
> fail instead to tell users that it is better to use an older version
> of pg_receivewal compatible with the build of this server. Better safe
> than sorry.

Hm, what are you proposing to do otherwise? I'm fairly unconvinced this
is a serious problem. It'll work in like 99.95% of all clusters, and in
the ones it won't work, it'll quickly afterwards, no? Forbidding
cross-version stuff for the large majority of installations that have
the default size, for the benefit the handfull of installations where
that's not the case? Not convinced.


> --- a/src/bin/pg_test_fsync/pg_test_fsync.c
> +++ b/src/bin/pg_test_fsync/pg_test_fsync.c
> @@ -64,7 +64,7 @@ static const char *progname;
> 
>  static int secs_per_test = 5;
>  static int needs_unlink = 0;
> -static char full_buf[XLOG_SEG_SIZE],
> +static char full_buf[DEFAULT_XLOG_SEG_SIZE],
> Did you consider adding an option --wal-segzize to pg_test_fsync?

Did, yes. Don't really think it's worthwhile. If somebody wants to come
up with a patch and argue for it, please...


> + * This is default value for wal_segment_size to be used at intidb when run
> Typo here: s/intidb/initdb/.

Ah, good catch.

Greetings,

Andres Freund


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