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

> ... 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-owner@postgresql.org
> [mailto:pgsql-hackers-owner@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-horikawa@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