Обсуждение: Re: [HACKERS] [COMMITTERS] pgsql: Make WAL segment size configurable at initdb time.

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.

--- 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?

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


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

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

On Wed, Sep 20, 2017 at 1:51 AM, Michael Paquier
<michael.paquier@gmail.com> wrote:
> 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.

I don't think this commit made anything any worse.  Before this
commit, the client used XLOG_SEG_SIZE anyway.  There was already no
guarantee that the value compiled into the client matched the value
compiled into the server to which it was connecting.  The difference
is, now we only assume that when the server is too old to tell us the
real answer, whereas before, we assumed it always.

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