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

From: Takashi Horikawa <t-horikawa(at)aj(dot)jp(dot)nec(dot)com>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: patch: prevent user from setting wal_buffers over 2GB bytes
Date: 2015-08-05 00:35:40
Message-ID: 73FA3881462C614096F815F75628AFCD0353A5BA@BPXM01GP.gisp.nec.co.jp
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

> ... 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(at)postgresql(dot)org
> [mailto:pgsql-hackers-owner(at)postgresql(dot)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(at)aj(dot)jp(dot)nec(dot)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(at)postgresql(dot)org) To make
> changes to your subscription:
> http://www.postgresql.org/mailpref/pgsql-hackers

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Josh Berkus 2015-08-05 00:46:41 Re: RFC: replace pg_stat_activity.waiting with something more descriptive
Previous Message Tom Lane 2015-08-05 00:24:56 Re: [sqlsmith] Failed assertion in joinrels.c