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

Hi Andres,

On 9/20/17 1:04 AM, Andres Freund wrote:

Make WAL segment size configurable at initdb time.

<...>

https://git.postgresql.org/pg/commitdiff/fc49e24fa69a15efacd5b8958115ed9c43c48f9a

It appears that fc49e24f missed updating the runtime config presets
documentation.

Patch attached.

Regards,
--
-David
david@pgmasters.net

On Thu, Sep 20, 2018 at 11:48:08AM -0400, David Steele wrote:

</term>
<listitem>
<para>
-        Reports the number of blocks (pages) in a WAL segment file.
-        The total size of a WAL segment file in bytes is equal to
-        <varname>wal_segment_size</varname> multiplied by <varname>wal_block_size</varname>;
-        by default this is 16MB.  See <xref linkend="wal-configuration"/> for
+        Reports the size of write ahead log segments.
+        The default value is 16MB. See <xref linkend="wal-configuration"/> for
more information.
</para>
</listitem>

The GUC wal_segment_size reports as well 16MB in v10 or older versions,
so shouldn't the mention to wal_block_size be removed even further down?
--
Michael

On 9/20/18 7:59 PM, Michael Paquier wrote:

On Thu, Sep 20, 2018 at 11:48:08AM -0400, David Steele wrote:

</term>
<listitem>
<para>
-        Reports the number of blocks (pages) in a WAL segment file.
-        The total size of a WAL segment file in bytes is equal to
-        <varname>wal_segment_size</varname> multiplied by <varname>wal_block_size</varname>;
-        by default this is 16MB.  See <xref linkend="wal-configuration"/> for
+        Reports the size of write ahead log segments.
+        The default value is 16MB. See <xref linkend="wal-configuration"/> for
more information.
</para>
</listitem>

The GUC wal_segment_size reports as well 16MB in v10 or older versions,
so shouldn't the mention to wal_block_size be removed even further down?

Oddly, the GUC reports 16GB on `show wal_segment_size` but you get 2048
when querying pg_settings. I thought that was just the normal behavior
for older versions -- and it might be a problem to change it.

--
-David
david@pgmasters.net

On 2018-09-20 21:14:51 -0400, David Steele wrote:

On 9/20/18 7:59 PM, Michael Paquier wrote:

On Thu, Sep 20, 2018 at 11:48:08AM -0400, David Steele wrote:

</term>
<listitem>
<para>
-        Reports the number of blocks (pages) in a WAL segment file.
-        The total size of a WAL segment file in bytes is equal to
-        <varname>wal_segment_size</varname> multiplied by <varname>wal_block_size</varname>;
-        by default this is 16MB.  See <xref linkend="wal-configuration"/> for
+        Reports the size of write ahead log segments.
+        The default value is 16MB. See <xref linkend="wal-configuration"/> for
more information.
</para>
</listitem>

The GUC wal_segment_size reports as well 16MB in v10 or older versions,
so shouldn't the mention to wal_block_size be removed even further down?

Oddly, the GUC reports 16GB on `show wal_segment_size` but you get 2048
when querying pg_settings. I thought that was just the normal behavior
for older versions -- and it might be a problem to change it.

16*M*B, right? If so, that's normal - pg_settings just reports the
values in the underlying unit - which is XLOG_BLCKSZ, compile-time
defaulting to 8KB. 8192 * 2048 = 16MB. That's the same in various other
settings.

Greetings,

Andres Freund

On Thu, Sep 20, 2018 at 06:23:54PM -0700, Andres Freund wrote:

16*M*B, right? If so, that's normal - pg_settings just reports the
values in the underlying unit - which is XLOG_BLCKSZ, compile-time
defaulting to 8KB. 8192 * 2048 = 16MB. That's the same in various other
settings.

Would it bring less confusion if we append something like "When querying
pg_settings"? I can see David's point the current phrasing is
confusing: we don't know if this comes from pg_settings or from SHOW.
It obviously refers to the former, but one can understand that it refers
to the latter.

A second parameter in config.sgml where this formulation is used is
segment_size.
--
Michael

On 9/21/18 12:53 AM, Michael Paquier wrote:

On Thu, Sep 20, 2018 at 06:23:54PM -0700, Andres Freund wrote:

16*M*B, right? If so, that's normal - pg_settings just reports the
values in the underlying unit - which is XLOG_BLCKSZ, compile-time
defaulting to 8KB. 8192 * 2048 = 16MB. That's the same in various other
settings.

Would it bring less confusion if we append something like "When querying
pg_settings"? I can see David's point the current phrasing is
confusing: we don't know if this comes from pg_settings or from SHOW.
It obviously refers to the former, but one can understand that it refers
to the latter.

A second parameter in config.sgml where this formulation is used is
segment_size.

That's why I used a default of 16MB instead of the byte value, because
segment_size used 1GB instead of the byte value.

--
-David
david@pgmasters.net

On 9/21/18 12:44 PM, David Steele wrote:

On 9/21/18 12:53 AM, Michael Paquier wrote:

On Thu, Sep 20, 2018 at 06:23:54PM -0700, Andres Freund wrote:

16*M*B, right? If so, that's normal - pg_settings just reports the
values in the underlying unit - which is XLOG_BLCKSZ, compile-time
defaulting to 8KB. 8192 * 2048 = 16MB. That's the same in various other
settings.

Would it bring less confusion if we append something like "When querying
pg_settings"? I can see David's point the current phrasing is
confusing: we don't know if this comes from pg_settings or from SHOW.
It obviously refers to the former, but one can understand that it refers
to the latter.

A second parameter in config.sgml where this formulation is used is
segment_size.

That's why I used a default of 16MB instead of the byte value, because
segment_size used 1GB instead of the byte value.

Not sure where we are on this, but for the record I still think the
description in PG11 needs to be corrected as in the patch. It doesn't
need to be back-patched and the default seems correct to me.

Thanks,
--
-David
david@pgmasters.net

On 2018-09-20 11:48:08 -0400, David Steele wrote:

Hi Andres,

On 9/20/17 1:04 AM, Andres Freund wrote:

Make WAL segment size configurable at initdb time.

<...>

https://git.postgresql.org/pg/commitdiff/fc49e24fa69a15efacd5b8958115ed9c43c48f9a

It appears that fc49e24f missed updating the runtime config presets
documentation.

Patch attached.

Regards,
--
-David
david@pgmasters.net

diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index e1073ac6d3..3bfd172441 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -8440,10 +8440,8 @@ dynamic_library_path = 'C:\tools\postgresql;H:\my_project\lib;$libdir'
</term>
<listitem>
<para>
-        Reports the number of blocks (pages) in a WAL segment file.
-        The total size of a WAL segment file in bytes is equal to
-        <varname>wal_segment_size</varname> multiplied by <varname>wal_block_size</varname>;
-        by default this is 16MB.  See <xref linkend="wal-configuration"/> for
+        Reports the size of write ahead log segments.
+        The default value is 16MB. See <xref linkend="wal-configuration"/> for
more information.
</para>
</listitem>

Why is this actually more correct? You mean because we have a conversion
that does the mb conversion at display time?

Greetings,

Andres Freund

Hi Andres,

On 10/5/18 5:54 PM, Andres Freund wrote:

On 2018-09-20 11:48:08 -0400, David Steele wrote:

diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index e1073ac6d3..3bfd172441 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -8440,10 +8440,8 @@ dynamic_library_path = 'C:\tools\postgresql;H:\my_project\lib;$libdir'
</term>
<listitem>
<para>
-        Reports the number of blocks (pages) in a WAL segment file.
-        The total size of a WAL segment file in bytes is equal to
-        <varname>wal_segment_size</varname> multiplied by <varname>wal_block_size</varname>;
-        by default this is 16MB.  See <xref linkend="wal-configuration"/> for
+        Reports the size of write ahead log segments.
+        The default value is 16MB. See <xref linkend="wal-configuration"/> for
more information.
</para>
</listitem>

Why is this actually more correct? You mean because we have a conversion
that does the mb conversion at display time?

In pre-11 versions of Postgres, you get this:

postgres=# select setting, unit from pg_settings where name =
'wal_segment_size';
setting | unit
---------+------
2048 | 8kB

But in v11 you get this:

select setting, unit from pg_settings where name = 'wal_segment_size';
setting | unit
----------+------
16777216 | B

So, while the WAL segment size used to be expressed in terms of 8K pages
it is now expressed in terms of absolute bytes. This seemed to me to be
a very deliberate change in the original commit so I guessed it was done
for clarity, but that the docs didn't get the message.

Regards,
--
-David
david@pgmasters.net

On 10/5/18 1:03 PM, David Steele wrote:

Hi Andres,

On 10/5/18 5:54 PM, Andres Freund wrote:

On 2018-09-20 11:48:08 -0400, David Steele wrote:

diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index e1073ac6d3..3bfd172441 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -8440,10 +8440,8 @@ dynamic_library_path =
'C:\tools\postgresql;H:\my_project\lib;$libdir'
        </term>
        <listitem>
         <para>
-        Reports the number of blocks (pages) in a WAL segment file.
-        The total size of a WAL segment file in bytes is equal to
-        <varname>wal_segment_size</varname> multiplied by
<varname>wal_block_size</varname>;
-        by default this is 16MB.  See <xref
linkend="wal-configuration"/> for
+        Reports the size of write ahead log segments.
+        The default value is 16MB. See <xref
linkend="wal-configuration"/> for
          more information.
         </para>
        </listitem>

Why is this actually more correct? You mean because we have a conversion
that does the mb conversion at display time?

In pre-11 versions of Postgres, you get this:

postgres=# select setting, unit from pg_settings where name =
'wal_segment_size';
 setting | unit
---------+------
 2048    | 8kB

But in v11 you get this:

select setting, unit from pg_settings where name = 'wal_segment_size';
 setting  | unit
----------+------
 16777216 | B

So, while the WAL segment size used to be expressed in terms of 8K pages
it is now expressed in terms of absolute bytes.  This seemed to me to be
a very deliberate change in the original commit so I guessed it was done
for clarity, but that the docs didn't get the message.

Thoughts on this?

I know it's minor in the grand scheme of things but it caused me some
confusion when I was updating pgBackRest for v11 and I imagine it might
do the same for others.

--
-David
david@pgmasters.net

On 2018-11-09 21:45:18 -0500, David Steele wrote:

On 10/5/18 1:03 PM, David Steele wrote:

Hi Andres,

On 10/5/18 5:54 PM, Andres Freund wrote:

On 2018-09-20 11:48:08 -0400, David Steele wrote:

diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index e1073ac6d3..3bfd172441 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -8440,10 +8440,8 @@ dynamic_library_path =
'C:\tools\postgresql;H:\my_project\lib;$libdir'
������� </term>
������� <listitem>
�������� <para>
-������� Reports the number of blocks (pages) in a WAL segment file.
-������� The total size of a WAL segment file in bytes is equal to
-������� <varname>wal_segment_size</varname> multiplied by
<varname>wal_block_size</varname>;
-������� by default this is 16MB.� See <xref
linkend="wal-configuration"/> for
+������� Reports the size of write ahead log segments.
+������� The default value is 16MB. See <xref
linkend="wal-configuration"/> for
��������� more information.
�������� </para>
������� </listitem>

Why is this actually more correct? You mean because we have a conversion
that does the mb conversion at display time?

In pre-11 versions of Postgres, you get this:

postgres=# select setting, unit from pg_settings where name =
'wal_segment_size';
�setting | unit
---------+------
�2048��� | 8kB

But in v11 you get this:

select setting, unit from pg_settings where name = 'wal_segment_size';
�setting� | unit
----------+------
�16777216 | B

So, while the WAL segment size used to be expressed in terms of 8K pages
it is now expressed in terms of absolute bytes.� This seemed to me to be
a very deliberate change in the original commit so I guessed it was done
for clarity, but that the docs didn't get the message.

Thoughts on this?

I know it's minor in the grand scheme of things but it caused me some
confusion when I was updating pgBackRest for v11 and I imagine it might
do the same for others.

Sorry for forgetting this, pushed.

I kinda wonder whether we should move a few GUCs out of the
<sect1 id="runtime-config-preset">
section. Or adapt its description. Because a significant portion of its
contents don't accurately seem to be described by
The following <quote>parameters</quote> are read-only, and are determined
when <productname>PostgreSQL</productname> is compiled or when it is
installed.
right?

Greetings,

Andres Freund

On 11/9/18 10:30 PM, Andres Freund wrote:

On 2018-11-09 21:45:18 -0500, David Steele wrote:

On 10/5/18 1:03 PM, David Steele wrote:

So, while the WAL segment size used to be expressed in terms of 8K pages
it is now expressed in terms of absolute bytes.  This seemed to me to be
a very deliberate change in the original commit so I guessed it was done
for clarity, but that the docs didn't get the message.

Thoughts on this?

I know it's minor in the grand scheme of things but it caused me some
confusion when I was updating pgBackRest for v11 and I imagine it might
do the same for others.

Sorry for forgetting this, pushed.

No worries. Thanks for pushing it.

I kinda wonder whether we should move a few GUCs out of the
<sect1 id="runtime-config-preset">
section. Or adapt its description. Because a significant portion of its
contents don't accurately seem to be described by
The following <quote>parameters</quote> are read-only, and are determined
when <productname>PostgreSQL</productname> is compiled or when it is
installed.
right?

I agree that "installed" is a bit vague.

Most of them are set at compile-time, some are set at initdb, and a few
are set at CREATE DATABASE.

I'm not sure about moving any out since it seems like "presets" fits the
bill. Maybe just break it up into three sections?

--
-David
david@pgmasters.net