Re: [HACKERS] patch: prevent user from setting wal_buffers over 2GB bytes

2015-08-04 Thread Tom Lane
Takashi Horikawa t-horik...@aj.jp.nec.com writes:
 Why does this cause a core dump?  We could consider fixing whatever
 the problem is rather than capping the value.

 As far as I experiment with my own evaluation environment using 
 PostgreSQL-9.4.4 on a x86_64 Linux, this problem can be fixed with the patch 
 attached.

I'm unsure whether this represents a complete fix ... but even if it does,
it would be awfully easy to re-introduce similar bugs in future code
changes, and who would notice?  Josh's approach of restricting the buffer
size seems a lot more robust.

If there were any practical use-case for such large WAL buffers then it
might be worth spending some effort/risk here.  But AFAICS, there is not.
Indeed, capping wal_buffers might be argued to be a good thing in itself
because it would prevent users from wasting shared memory foolishly.

So my vote is for the original approach.  (I've not read Josh's patch,
so there might be something wrong with it in detail, but I like the
basic approach.)

regards, tom lane


-- 
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] patch: prevent user from setting wal_buffers over 2GB bytes

2015-08-04 Thread Andres Freund
On 2015-08-04 09:49:58 -0400, Tom Lane wrote:
 Takashi Horikawa t-horik...@aj.jp.nec.com writes:
  Why does this cause a core dump?  We could consider fixing whatever
  the problem is rather than capping the value.
 
  As far as I experiment with my own evaluation environment using 
  PostgreSQL-9.4.4 on a x86_64 Linux, this problem can be fixed with the 
  patch 
  attached.
 
 I'm unsure whether this represents a complete fix ... but even if it does,
 it would be awfully easy to re-introduce similar bugs in future code
 changes, and who would notice?  Josh's approach of restricting the buffer
 size seems a lot more robust.
 
 If there were any practical use-case for such large WAL buffers then it
 might be worth spending some effort/risk here.  But AFAICS, there is not.
 Indeed, capping wal_buffers might be argued to be a good thing in itself
 because it would prevent users from wasting shared memory foolishly.
 
 So my vote is for the original approach.  (I've not read Josh's patch,
 so there might be something wrong with it in detail, but I like the
 basic approach.)

+1


-- 
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] patch: prevent user from setting wal_buffers over 2GB bytes

2015-08-04 Thread Robert Haas
On Tue, Aug 4, 2015 at 9:52 AM, Andres Freund and...@anarazel.de wrote:
 So my vote is for the original approach.  (I've not read Josh's patch,
 so there might be something wrong with it in detail, but I like the
 basic approach.)

 +1

OK, committed and back-patched that all the way back to 9.0.

-- 
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] patch: prevent user from setting wal_buffers over 2GB bytes

2015-08-04 Thread Takashi Horikawa
 ... Josh's approach of restricting the buffer size seems
 a lot more robust.
I understand that the capping of approach of restricting the buffer size
is much more robust and is suitable in this case.

I, howerver, think that the chane from
'page = XLogCtl-pages[firstIdx * XLOG_BLCKSZ];'
to
'page = XLogCtl-pages[firstIdx * (Size) XLOG_BLCKSZ];'
is no harm even when restricting the wal buffer size.

It is in harmony with the usage of 'XLogCtl-pages' found in, for example, 
'cachedPos = XLogCtl-pages + idx * (Size) XLOG_BLCKSZ;'
in GetXLogBuffer(XLogRecPtr ptr)
and 
'NewPage = (XLogPageHeader) (XLogCtl-pages + nextidx * (Size) XLOG_BLCKSZ);
'
in AdvanceXLInsertBuffer(XLogRecPtr upto, bool opportunistic)
, etc.

Only exception is
'page = XLogCtl-pages[firstIdx * XLOG_BLCKSZ];'
in
StartupXLOG(void)
--
Takashi Horikawa
NEC Corporation
Knowledge Discovery Research Laboratories


 -Original Message-
 From: pgsql-hackers-ow...@postgresql.org
 [mailto:pgsql-hackers-ow...@postgresql.org] On Behalf Of Tom Lane
 Sent: Tuesday, August 04, 2015 10:50 PM
 To: Horikawa Takashi(堀川 隆)
 Cc: PostgreSQL-development
 Subject: Re: [HACKERS] patch: prevent user from setting wal_buffers over
 2GB bytes
 
 Takashi Horikawa t-horik...@aj.jp.nec.com writes:
  Why does this cause a core dump?  We could consider fixing whatever
  the problem is rather than capping the value.
 
  As far as I experiment with my own evaluation environment using
  PostgreSQL-9.4.4 on a x86_64 Linux, this problem can be fixed with the
  patch attached.
 
 I'm unsure whether this represents a complete fix ... but even if it does,
 it would be awfully easy to re-introduce similar bugs in future code
changes,
 and who would notice?  Josh's approach of restricting the buffer size
seems
 a lot more robust.
 
 If there were any practical use-case for such large WAL buffers then it
 might be worth spending some effort/risk here.  But AFAICS, there is not.
 Indeed, capping wal_buffers might be argued to be a good thing in itself
 because it would prevent users from wasting shared memory foolishly.
 
 So my vote is for the original approach.  (I've not read Josh's patch, so
 there might be something wrong with it in detail, but I like the basic
 approach.)
 
   regards, tom lane
 
 
 --
 Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make
 changes to your subscription:
 http://www.postgresql.org/mailpref/pgsql-hackers


smime.p7s
Description: S/MIME cryptographic signature


Re: [HACKERS] patch: prevent user from setting wal_buffers over 2GB bytes

2015-08-03 Thread Takashi Horikawa
  Why does this cause a core dump?  We could consider fixing whatever
  the problem is rather than capping the value.
As far as I experiment with my own evaluation environment using 
PostgreSQL-9.4.4 on a x86_64 Linux, this problem can be fixed with the patch 
attached.

I have confirmed that applying the patch makes 'wal_buffers = 4GB'  works 
fine, while original PostgreSQL-9.4.4 results in core dump (segfault). I'll be 
happy if anyone reconfirm this.

--
Takashi Horikawa
NEC Corporation
Knowledge Discovery Research Laboratories


 -Original Message-
 From: pgsql-hackers-ow...@postgresql.org
 [mailto:pgsql-hackers-ow...@postgresql.org] On Behalf Of Robert Haas
 Sent: Tuesday, August 04, 2015 2:29 AM
 To: Josh Berkus
 Cc: PostgreSQL-development
 Subject: Re: [HACKERS] patch: prevent user from setting wal_buffers over
 2GB bytes

 On Fri, Jul 31, 2015 at 8:09 PM, Josh Berkus j...@agliodbs.com wrote:
  On 07/31/2015 10:43 AM, Robert Haas wrote:
  On Thu, Jul 30, 2015 at 9:17 PM, Josh Berkus j...@agliodbs.com wrote:
  In guc.c, the maximum for wal_buffers is INT_MAX.  However,
  wal_buffers is actually measured in 8KB buffers, not in bytes.  This
  means that users are able to set wal_buffers  2GB.  When the
  database is started, this can cause a core dump if the WAL offset is
  2GB.
 
  Why does this cause a core dump?  We could consider fixing whatever
  the problem is rather than capping the value.
 
  The underlying issue is that byte position in wal_buffers is a 32-bit
  INT, so as soon as you exceed that, core dump.

 OK.  So capping it sounds like the right approach, then.

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


allow_wal_buffer_over_2G.patch
Description: Binary data


smime.p7s
Description: S/MIME cryptographic signature


Re: [HACKERS] patch: prevent user from setting wal_buffers over 2GB bytes

2015-08-03 Thread Robert Haas
On Fri, Jul 31, 2015 at 8:09 PM, Josh Berkus j...@agliodbs.com wrote:
 On 07/31/2015 10:43 AM, Robert Haas wrote:
 On Thu, Jul 30, 2015 at 9:17 PM, Josh Berkus j...@agliodbs.com wrote:
 In guc.c, the maximum for wal_buffers is INT_MAX.  However, wal_buffers
 is actually measured in 8KB buffers, not in bytes.  This means that
 users are able to set wal_buffers  2GB.  When the database is started,
 this can cause a core dump if the WAL offset is  2GB.

 Why does this cause a core dump?  We could consider fixing whatever
 the problem is rather than capping the value.

 The underlying issue is that byte position in wal_buffers is a 32-bit
 INT, so as soon as you exceed that, core dump.

OK.  So capping it sounds like the right approach, then.

-- 
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] patch: prevent user from setting wal_buffers over 2GB bytes

2015-07-31 Thread Josh Berkus
On 07/31/2015 10:43 AM, Robert Haas wrote:
 On Thu, Jul 30, 2015 at 9:17 PM, Josh Berkus j...@agliodbs.com wrote:
 In guc.c, the maximum for wal_buffers is INT_MAX.  However, wal_buffers
 is actually measured in 8KB buffers, not in bytes.  This means that
 users are able to set wal_buffers  2GB.  When the database is started,
 this can cause a core dump if the WAL offset is  2GB.
 
 Why does this cause a core dump?  We could consider fixing whatever
 the problem is rather than capping the value.

The underlying issue is that byte position in wal_buffers is a 32-bit
INT, so as soon as you exceed that, core dump.

Given that useful ranges for wal_buffers are in the 8MB to 128MB range,
I don't see that a cap at 2GB is much of a burden.  For that reason, I'd
prefer capping the value to replumbing.  We have not previously
documented the  limit for this parameter, which is probably how the bug
happened in the first place.  Clearly no user has been setting it above
2GB, or they would have been core dumping.

Oh, and yes, I think this should be backported; this issue exists in all
supported versions.

-- 
Josh Berkus
PostgreSQL Experts Inc.
http://pgexperts.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] patch: prevent user from setting wal_buffers over 2GB bytes

2015-07-31 Thread Robert Haas
On Thu, Jul 30, 2015 at 9:17 PM, Josh Berkus j...@agliodbs.com wrote:
 In guc.c, the maximum for wal_buffers is INT_MAX.  However, wal_buffers
 is actually measured in 8KB buffers, not in bytes.  This means that
 users are able to set wal_buffers  2GB.  When the database is started,
 this can cause a core dump if the WAL offset is  2GB.

Why does this cause a core dump?  We could consider fixing whatever
the problem is rather than capping the value.

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