Re: [HACKERS] checkpoint_flush_after documentation inconsistency

[Magnus Hagander]

On Mon, Apr 25, 2016 at 6:57 AM, Fujii Masao  wrote:

> On Mon, Apr 25, 2016 at 4:27 AM, Andres Freund  wrote:
> > On 2016-04-21 11:20:38 -0700, Andres Freund wrote:
> >> On 2016-04-18 14:33:28 +0900, Fujii Masao wrote:
> >> > On Fri, Apr 15, 2016 at 6:56 PM, Magnus Hagander 
> wrote:
> >> > > The documentation says that the default value is 128Kb on Linux,
> but the
> >> > > code says it's 256Kb.
> >> > >
> >> > > Not sure which one is correct, but the other one should be updated
> :) I'm
> >> > > guessing it's a copy/paste mistake in the docs, but since I'm not
> sure I'm
> >> > > not just pushing a fix.
> >> >
> >> > I think you're right.
> >> >
> >> > I also found another several small problems regarding XXX_flush_after
> >> > parameters.
> >>
> >> Thanks for finding these, once I'm back home/office from pgconf.nyc
> >> (tonight / tomorrow) I'll push a fix.
> >
> > Pushed a fix, could you check that you're ok with the result?
>
> The fix looks good to me. Thanks!
>
>
+1, looks good.


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

Re: [HACKERS] checkpoint_flush_after documentation inconsistency

[Fujii Masao]

On Mon, Apr 25, 2016 at 4:27 AM, Andres Freund  wrote:
> On 2016-04-21 11:20:38 -0700, Andres Freund wrote:
>> On 2016-04-18 14:33:28 +0900, Fujii Masao wrote:
>> > On Fri, Apr 15, 2016 at 6:56 PM, Magnus Hagander  
>> > wrote:
>> > > The documentation says that the default value is 128Kb on Linux, but the
>> > > code says it's 256Kb.
>> > >
>> > > Not sure which one is correct, but the other one should be updated :) I'm
>> > > guessing it's a copy/paste mistake in the docs, but since I'm not sure 
>> > > I'm
>> > > not just pushing a fix.
>> >
>> > I think you're right.
>> >
>> > I also found another several small problems regarding XXX_flush_after
>> > parameters.
>>
>> Thanks for finding these, once I'm back home/office from pgconf.nyc
>> (tonight / tomorrow) I'll push a fix.
>
> Pushed a fix, could you check that you're ok with the result?

The fix looks good to me. Thanks!

Regards,

-- 
Fujii Masao


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

Re: [HACKERS] checkpoint_flush_after documentation inconsistency

[Andres Freund]

On 2016-04-21 11:20:38 -0700, Andres Freund wrote:
> On 2016-04-18 14:33:28 +0900, Fujii Masao wrote:
> > On Fri, Apr 15, 2016 at 6:56 PM, Magnus Hagander  
> > wrote:
> > > The documentation says that the default value is 128Kb on Linux, but the
> > > code says it's 256Kb.
> > >
> > > Not sure which one is correct, but the other one should be updated :) I'm
> > > guessing it's a copy/paste mistake in the docs, but since I'm not sure I'm
> > > not just pushing a fix.
> > 
> > I think you're right.
> > 
> > I also found another several small problems regarding XXX_flush_after
> > parameters.
> 
> Thanks for finding these, once I'm back home/office from pgconf.nyc
> (tonight / tomorrow) I'll push a fix.

Pushed a fix, could you check that you're ok with the result?

Also started 
http://archives.postgresql.org/message-id/20160424185814.n3nj2ahlutoykfou%40
to discuss making the ordering of config variables more consistent.

Thanks Magnus and Fujii

Andres


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

Re: [HACKERS] checkpoint_flush_after documentation inconsistency

[Andres Freund]

On 2016-04-22 23:33:07 -0400, Robert Haas wrote:
> On Thu, Apr 21, 2016 at 2:20 PM, Andres Freund  wrote:
> > On 2016-04-18 14:33:28 +0900, Fujii Masao wrote:
> >> On Fri, Apr 15, 2016 at 6:56 PM, Magnus Hagander  
> >> wrote:
> >> > The documentation says that the default value is 128Kb on Linux, but the
> >> > code says it's 256Kb.
> >> >
> >> > Not sure which one is correct, but the other one should be updated :) I'm
> >> > guessing it's a copy/paste mistake in the docs, but since I'm not sure 
> >> > I'm
> >> > not just pushing a fix.
> >>
> >> I think you're right.
> >>
> >> I also found another several small problems regarding XXX_flush_after
> >> parameters.
> >
> > Thanks for finding these, once I'm back home/office from pgconf.nyc
> > (tonight / tomorrow) I'll push a fix.
> 
> Ping.

Working on a fix for invalidation issue [1] right now, because I want to
see that fixed before pushing the fix for the smgr thing [2]. Will get
to this afterwards.

Andres

[1] 
http://www.postgresql.org/message-id/CAD21AoDpZ6Xjg=gFrGPnSn4oTRRcwK1EBrWCq9OqOHuAcMMC=w...@mail.gmail.com
[2] 
http://archives.postgresql.org/message-id/CAA-aLv6Dp_ZsV-44QA-2zgkqWKQq%3DGedBX2dRSrWpxqovXK%3DPg%40mail.gmail.com


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

Re: [HACKERS] checkpoint_flush_after documentation inconsistency

[Robert Haas]

On Thu, Apr 21, 2016 at 2:20 PM, Andres Freund  wrote:
> On 2016-04-18 14:33:28 +0900, Fujii Masao wrote:
>> On Fri, Apr 15, 2016 at 6:56 PM, Magnus Hagander  wrote:
>> > The documentation says that the default value is 128Kb on Linux, but the
>> > code says it's 256Kb.
>> >
>> > Not sure which one is correct, but the other one should be updated :) I'm
>> > guessing it's a copy/paste mistake in the docs, but since I'm not sure I'm
>> > not just pushing a fix.
>>
>> I think you're right.
>>
>> I also found another several small problems regarding XXX_flush_after
>> parameters.
>
> Thanks for finding these, once I'm back home/office from pgconf.nyc
> (tonight / tomorrow) I'll push a fix.

Ping.

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

Re: [HACKERS] checkpoint_flush_after documentation inconsistency

[Andres Freund]

On 2016-04-18 14:33:28 +0900, Fujii Masao wrote:
> On Fri, Apr 15, 2016 at 6:56 PM, Magnus Hagander  wrote:
> > The documentation says that the default value is 128Kb on Linux, but the
> > code says it's 256Kb.
> >
> > Not sure which one is correct, but the other one should be updated :) I'm
> > guessing it's a copy/paste mistake in the docs, but since I'm not sure I'm
> > not just pushing a fix.
> 
> I think you're right.
> 
> I also found another several small problems regarding XXX_flush_after
> parameters.

Thanks for finding these, once I'm back home/office from pgconf.nyc
(tonight / tomorrow) I'll push a fix.

Andres


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

Re: [HACKERS] checkpoint_flush_after documentation inconsistency

[Fujii Masao]

On Fri, Apr 15, 2016 at 6:56 PM, Magnus Hagander  wrote:
> The documentation says that the default value is 128Kb on Linux, but the
> code says it's 256Kb.
>
> Not sure which one is correct, but the other one should be updated :) I'm
> guessing it's a copy/paste mistake in the docs, but since I'm not sure I'm
> not just pushing a fix.

I think you're right.

I also found another several small problems regarding XXX_flush_after
parameters.

(1)
checkpoint_flush_after, backend_flush_after and bgwriter_flush_after
don't exist in postgresql.conf.sample. If there is no special reason for
that, it's better to add them to postgresql.conf.sample.

(2)
> /* see bufmgr.h: 16 on Linux, 0 otherwise */

The source code comment says that the default value of bgwriter_flush_after
is 16 on Linux, but it's actually 64. The parameter which its default is 16 is
backend_flush_after. This souce comment needs to be updated.

(3)
The config groups assigned to them look strange.

The group of checkpoint_flush_after is RESOURCES_ASYNCHRONOUS,
but it's documented under the section "Checkpoints". IMO, it's better
to change the group to WAL_CHECKPOINTS.

The group of bgwriter_flush_after is WAL_CHECKPOINTS,
but it's documented under the section Background Writer. IMO,
it's better to change the group to RESOURCES_BGWRITER.

(4)
> This parameter can only be set in the postgresql.conf file or on
> the server command line.

The above description should be used for a PGC_SIGHUP parameter.
But it's used for backend_flush_after which is PGC_USERSET parameter.

(5)
bgwriter_flush_after (int)
backend_flush_after (int)
checkpoint_flush_after (int)

In the doc, "int" should be "integer" for the sake of consistency.

Regards,

-- 
Fujii Masao


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