Re: Better handling of archive_command problems - Mailing list pgsql-hackers

On Wed, May 15, 2013 at 3:46 AM, Robert Haas <robertmhaas@gmail.com> wrote:
> One possible objection to this line of attack is that, IIUC, waits to
> acquire a LWLock are non-interruptible.  If someone tells PostgreSQL
> to wait for some period of time before performing each WAL write,
> other backends that grab the WALWriteLock will not respond to query
> cancels during that time.

I don't see any reasonable way to make LWLocks care about interrupts
(using all 3 possible underlying semaphore implementations, no less).
As it says within LWLockAcquire:

/** Lock out cancel/die interrupts until we exit the code section protected* by the LWLock.  This ensures that
interruptswill not interfere with* manipulations of data structures in shared memory.*/
 
HOLD_INTERRUPTS();

We've been pretty judicious about placing CHECK_FOR_INTERRUPTS() calls
in the right places, but it's still quite possible to see the server
take multiple seconds - perhaps even as many as 10 - to respond to an
interrupt (by psql SIGINT). Now, I didn't have enough of an interest
at the times I noticed this to figure out exactly why that may have
been or to somehow characterize it, but I don't accept that it's a
violation of some Postgres precept that this setting could result in
interrupts taking multiple seconds, and maybe even as many as 10. I'd
go so far as to let the user make the throttling sleep take as long as
they like, even though this admittedly would sort of break such a
precept.

There is a setting called zero_damaged_pages, and enabling it causes
data loss. I've seen cases where it was enabled within postgresql.conf
for years.

> Worse, the locks have a tendency to back up.

Well, yes, sleeping on WALWriteLock has some fairly bad consequences
for performance. That's sort of the idea. The bar is rather low here,
to my mind (the state of the art currently is deal with it/monitor it
yourself, and risk a PANIC shutdown if you fail). I think it's worth
noting that at least one other "Enterprise" RDBMS has a similar
feature.

The only situation that I can see where a backend would acquire
WALWriteLock alongside WALInsertLock is when a segment switch needs to
occur (i.e. a XLOG_SWITCH record is being inserted). If that's the
case, no one is going to be able to insert WAL for as long as the
sleep to throttle occurs anyway, so there is no additional harm done.

> Now despite all that, I can see this being useful enough that Heroku
> might want to insert a very small patch into their version of
> PostgreSQL to do it this way, and just live with the downsides.  But
> anything that can propagate non-interruptible waits across the entire
> system does not sound to me like a feature that is sufficiently
> polished that we want to expose it to users less sophisticated than
> Heroku (i.e. nearly all of them).  If we do this, I think we ought to
> find a way to make the waits interruptible, and to insert them in
> places where they really don't interfere with read-only backends.

It would be nice to be able to be sure that CLogControlLock could not
be held for multiple seconds as a result of this. However, I don't see
any reasons to let the perfect be the enemy of the good, or at least
the better. Just how likely is it that the scenario you describe will
affect reads in the real world? In any case, this is a problem in its
own right.

I don't intend to make any promises about how this throttling will
affect read queries, except perhaps something vague and loose.

> I'd probably also argue that we ought to try to design it such that the
> GUC can be in MB/s rather than delay/WAL writer cycle.

That'd certainly be better, but I can't see a way of doing it without
adding a whole bunch of mechanism to some important codepaths, like
within XLogWrite(), which would be quite a hard sell.

-- 
Peter Geoghegan