Обсуждение: [Patch] change the default value of shared_buffers in postgresql.conf.sample

Hi all

In PostgreSQL 14, The default value of shared_buffers is 128MB, but in postgresql.conf.sample, the default value of
shared_buffersis 32MB.
 

I think the following changes should be made.

File: postgresql\src\backend\utils\misc\ postgresql.conf.sample
#shared_buffers = 32MB  =>  #shared_buffers = 128MB

[PostgreSQL 14]
shared_buffers (integer)
Sets the amount of memory the database server uses for shared memory buffers. The default is typically 128 megabytes
(128MB)
https://www.postgresql.org/docs/14/runtime-config-resource.html

--------------------------------------------------------------------
In PostgreSQL 9.2, The default value of shared_buffers is 32MB.

[PostgreSQL 9.2]
shared_buffers (integer)
Sets the amount of memory the database server uses for shared memory buffers. The default is typically 32 megabytes
(32MB)
https://www.postgresql.org/docs/9.2/runtime-config-resource.html

Here is a patch.

Best Regards!

"zhangjie2@fujitsu.com" <zhangjie2@fujitsu.com> writes:
> In PostgreSQL 14, The default value of shared_buffers is 128MB, but in postgresql.conf.sample, the default value of
shared_buffersis 32MB. 
> I think the following changes should be made.

> File: postgresql\src\backend\utils\misc\ postgresql.conf.sample
> #shared_buffers = 32MB  =>  #shared_buffers = 128MB

As submitted, this patch breaks initdb, which is looking for the exact
string "#shared_buffers = 32MB".

We could adjust that too of course, but I'm dubious first that any
change is needed, and second that this is the right one:

1. Since initdb will replace that string, users will never see this
entry as-is in live databases.  So is it worth doing anything?

2. The *actual*, hard-wired, default in guc.c is 1024 (8MB), not
either of these numbers.  So maybe the sample file ought to use
that instead.  Or maybe we should change that value too ... it's
surely as obsolete as can be.

On the whole this seems pretty cosmetic so I'm inclined to leave
it alone.  But if we were going to do anything I think that
adjusting both initdb.c and guc.c to use 128MB might be the
most appropriate thing.

            regards, tom lane

> On 24 Jun 2021, at 17:49, Tom Lane <tgl@sss.pgh.pa.us> wrote:

> ..if we were going to do anything I think that adjusting both initdb.c and
> guc.c to use 128MB might be the most appropriate thing.


Ensuring consistency doesn't seem like a bad thing in itself, even if it in
practice won't make much difference.

--
Daniel Gustafsson        https://vmware.com/

> On the whole this seems pretty cosmetic so I'm inclined to leave it alone.  But if we were going to do anything I
thinkthat adjusting both initdb.c and guc.c to use 128MB might be the most appropriate thing.
 

Thank you for your suggestions. initdb.c and guc.c have been modified together.

Best Regards!
Zhangjie

-----Original Message-----
From: Tom Lane <tgl@sss.pgh.pa.us> 
Sent: Thursday, June 24, 2021 11:50 PM
To: Zhang, Jie/张 杰 <zhangjie2@fujitsu.com>
Cc: pgsql-hackers@lists.postgresql.org
Subject: Re: [Patch] change the default value of shared_buffers in postgresql.conf.sample

"zhangjie2@fujitsu.com" <zhangjie2@fujitsu.com> writes:
> In PostgreSQL 14, The default value of shared_buffers is 128MB, but in postgresql.conf.sample, the default value of
shared_buffersis 32MB.
 
> I think the following changes should be made.

> File: postgresql\src\backend\utils\misc\ postgresql.conf.sample 
> #shared_buffers = 32MB  =>  #shared_buffers = 128MB

As submitted, this patch breaks initdb, which is looking for the exact string "#shared_buffers = 32MB".

We could adjust that too of course, but I'm dubious first that any change is needed, and second that this is the right
one:

1. Since initdb will replace that string, users will never see this entry as-is in live databases.  So is it worth
doinganything?
 

2. The *actual*, hard-wired, default in guc.c is 1024 (8MB), not either of these numbers.  So maybe the sample file
oughtto use that instead.  Or maybe we should change that value too ... it's surely as obsolete as can be.
 

On the whole this seems pretty cosmetic so I'm inclined to leave it alone.  But if we were going to do anything I think
thatadjusting both initdb.c and guc.c to use 128MB might be the most appropriate thing.
 

            regards, tom lane

On Thu, Jun 24, 2021 at 5:49 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>
> "zhangjie2@fujitsu.com" <zhangjie2@fujitsu.com> writes:
> > In PostgreSQL 14, The default value of shared_buffers is 128MB, but in postgresql.conf.sample, the default value of
shared_buffersis 32MB.
 
> > I think the following changes should be made.
>
> > File: postgresql\src\backend\utils\misc\ postgresql.conf.sample
> > #shared_buffers = 32MB  =>  #shared_buffers = 128MB
>
> As submitted, this patch breaks initdb, which is looking for the exact
> string "#shared_buffers = 32MB".
>
> We could adjust that too of course, but I'm dubious first that any
> change is needed, and second that this is the right one:
>
> 1. Since initdb will replace that string, users will never see this
> entry as-is in live databases.  So is it worth doing anything?

It's not entirely uncommon that users copy the .sample file into their
configuration management system and then generate the real config from
that using templates. These users will definitely see it (and
overwrite it).


> 2. The *actual*, hard-wired, default in guc.c is 1024 (8MB), not
> either of these numbers.  So maybe the sample file ought to use
> that instead.  Or maybe we should change that value too ... it's
> surely as obsolete as can be.

+1 for changing this one as well. It'a always been slightly confusing,
since it's what shows up in pg_settings. If anything I'd consider that
an oversight when the defaults were changed back then...


> On the whole this seems pretty cosmetic so I'm inclined to leave
> it alone.  But if we were going to do anything I think that
> adjusting both initdb.c and guc.c to use 128MB might be the
> most appropriate thing.

It is mostly cosmetic, but it is cosmetic at a level that can cause at
least a small amount of confusion for users, so I'm definitely +1 for
cleaning it up.

-- 
 Magnus Hagander
 Me: https://www.hagander.net/
 Work: https://www.redpill-linpro.com/

On Tue, Jun 29, 2021 at 09:28:04AM +0200, Magnus Hagander wrote:
> > 1. Since initdb will replace that string, users will never see this
> > entry as-is in live databases.  So is it worth doing anything?
> 
> It's not entirely uncommon that users copy the .sample file into their
> configuration management system and then generate the real config from
> that using templates. These users will definitely see it (and
> overwrite it).
> 
> 
> > 2. The *actual*, hard-wired, default in guc.c is 1024 (8MB), not
> > either of these numbers.  So maybe the sample file ought to use
> > that instead.  Or maybe we should change that value too ... it's
> > surely as obsolete as can be.
> 
> +1 for changing this one as well. It'a always been slightly confusing,
> since it's what shows up in pg_settings. If anything I'd consider that
> an oversight when the defaults were changed back then...
> 
> 
> > On the whole this seems pretty cosmetic so I'm inclined to leave
> > it alone.  But if we were going to do anything I think that
> > adjusting both initdb.c and guc.c to use 128MB might be the
> > most appropriate thing.
> 
> It is mostly cosmetic, but it is cosmetic at a level that can cause at
> least a small amount of confusion for users, so I'm definitely +1 for
> cleaning it up.

Yes, I liked this patch from June for the reasons outlined above.  I
don't think there is any logic to why pg_settings shows 8MB and
postgresql.conf.sample has 32MB, and this has been true since PG 10.

I think the only question is whether this is a PG 15-only patch or a
patckpatch to PG 10;  I am in favor of the later.

-- 
  Bruce Momjian  <bruce@momjian.us>        https://momjian.us
  EDB                                      https://enterprisedb.com

  If only the physical world exists, free will is an illusion.

Bruce Momjian <bruce@momjian.us> writes:
> I think the only question is whether this is a PG 15-only patch or a
> patckpatch to PG 10;  I am in favor of the later.

I think you need a lot stronger argument that this is a bug
before you consider back-patching user-visible behavioral
changes.

            regards, tom lane

On Wed, Aug 18, 2021 at 02:03:56PM -0400, Tom Lane wrote:
> Bruce Momjian <bruce@momjian.us> writes:
> > I think the only question is whether this is a PG 15-only patch or a
> > patckpatch to PG 10;  I am in favor of the later.
> 
> I think you need a lot stronger argument that this is a bug
> before you consider back-patching user-visible behavioral
> changes.

I think the only logic to backpatching it is your statement that this is
cosmetic, and the new cosmetic appearance is more accurate.  However, if
you don't feel we need to backpatch, that is fine with me --- we have
gotten very few complaints about this.

-- 
  Bruce Momjian  <bruce@momjian.us>        https://momjian.us
  EDB                                      https://enterprisedb.com

  If only the physical world exists, free will is an illusion.

On Wed, Aug 18, 2021 at 8:16 PM Bruce Momjian <bruce@momjian.us> wrote:
>
> On Wed, Aug 18, 2021 at 02:03:56PM -0400, Tom Lane wrote:
> > Bruce Momjian <bruce@momjian.us> writes:
> > > I think the only question is whether this is a PG 15-only patch or a
> > > patckpatch to PG 10;  I am in favor of the later.
> >
> > I think you need a lot stronger argument that this is a bug
> > before you consider back-patching user-visible behavioral
> > changes.
>
> I think the only logic to backpatching it is your statement that this is
> cosmetic, and the new cosmetic appearance is more accurate.  However, if
> you don't feel we need to backpatch, that is fine with me --- we have
> gotten very few complaints about this.

+1 for making the change ,and +1 for making it in master only, no backpatch.

-- 
 Magnus Hagander
 Me: https://www.hagander.net/
 Work: https://www.redpill-linpro.com/

On Wed, Aug 18, 2021 at 08:27:19PM +0200, Magnus Hagander wrote:
> On Wed, Aug 18, 2021 at 8:16 PM Bruce Momjian <bruce@momjian.us> wrote:
> >
> > On Wed, Aug 18, 2021 at 02:03:56PM -0400, Tom Lane wrote:
> > > Bruce Momjian <bruce@momjian.us> writes:
> > > > I think the only question is whether this is a PG 15-only patch or a
> > > > patckpatch to PG 10;  I am in favor of the later.
> > >
> > > I think you need a lot stronger argument that this is a bug
> > > before you consider back-patching user-visible behavioral
> > > changes.
> >
> > I think the only logic to backpatching it is your statement that this is
> > cosmetic, and the new cosmetic appearance is more accurate.  However, if
> > you don't feel we need to backpatch, that is fine with me --- we have
> > gotten very few complaints about this.
> 
> +1 for making the change ,and +1 for making it in master only, no backpatch.

Patch applied to master.

-- 
  Bruce Momjian  <bruce@momjian.us>        https://momjian.us
  EDB                                      https://enterprisedb.com

  If only the physical world exists, free will is an illusion.