Re: [HACKERS] checkpoint_flush_after documentation inconsistency

2016-04-25 Thread 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

2016-04-24 Thread 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

2016-04-24 Thread 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

2016-04-22 Thread 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

2016-04-22 Thread 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

2016-04-21 Thread 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

2016-04-17 Thread 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


[HACKERS] checkpoint_flush_after documentation inconsistency

2016-04-15 Thread Magnus Hagander
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.

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