Re: Less than ideal error reporting in pg_stat_statements

From: Peter Geoghegan <pg(at)heroku(dot)com>
To: Jim Nasby <Jim(dot)Nasby(at)bluetreble(dot)com>
Cc: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Pg Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Less than ideal error reporting in pg_stat_statements
Date: 2015-10-05 01:03:48
Message-ID: CAM3SWZRr6WqO7ZZc-EfuZU97TN6wcSiXvi8mRZe6ZGY6WCz+Mw@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Wed, Sep 23, 2015 at 1:41 PM, Jim Nasby <Jim(dot)Nasby(at)bluetreble(dot)com> wrote:
> max was set to 10000. I don't know about average query text size, but the
> command that was causing the error was a very large number of individual
> INSERT ... VALUES statements all in one command.
>
> The machine had plenty of free memory and no ulimit, so I don't see how this
> could have been anything but MaxAllocSize, unless there's some other failure
> mode in malloc I don't know about.

The patch that Tom committed does __nothing__ to actually address
*how* you were able to get into this position in the first place.
Sure, now you might be able to recover if you're not affected by the
MaxAllocSize limitation, but: What did it take in the first place to
need memory > MaxAllocSize to do a garbage collection? Whatever it was
that allowed things to get that bad before a garbage collection was
attempted is not even considered in the patch committed. The patch
just committed is, in short, nothing more than a band-aid, and may do
more harm than good due to allocating huge amounts of memory while a
very contended exclusive LWLock is held.

You say you have plenty of memory and no ulimit, and I believe that
you're right that there was never a conventional OOM where malloc()
returns NULL. If we conservatively assume that you were only barely
over the MaxAllocSize limitation before a garbage collection was first
attempted, then the average query text length would have to be about
50 kilobytes, since only then 20,000 texts (pg_stat_statements.max *
2) would put us over.

Does anyone actually think that the *average* SQL query on Jim's
client's application was in excess of 50% the size of the ~3,000 line
file pg_stat_statements.c? It beggars belief.

I'm annoyed and disappointed that the patch committed does not even
begin to address the underlying problem -- it just adds an escape
hatch, and fixes another theoretical issue that no one was affected
by. Honestly, next time I won't bother.

--
Peter Geoghegan

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Jeff Janes 2015-10-05 01:04:17 Re: Speed up Clog Access by increasing CLOG buffers
Previous Message Kouhei Kaigai 2015-10-05 00:51:02 Re: CustomScan support on readfuncs.c