Re: [COMMITTERS] pgsql: Make WAL segment size configurable at initdb time.

On 2017-09-20 14:51:45 +0900, Michael Paquier wrote:
> On Wed, Sep 20, 2017 at 2:04 PM, Andres Freund <andres(at)anarazel(dot)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