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