Re: [HACKERS] shared memory based stat collector (was: Sharing recordtypmods between backends) - Mailing list pgsql-hackers

From Robert Haas
Subject Re: [HACKERS] shared memory based stat collector (was: Sharing recordtypmods between backends)
Date
Msg-id CA+TgmoaYofBKKXBUcfgb9Rd5cKvZL7zhqHBbnUnNO+sdGpeVpA@mail.gmail.com
Whole thread Raw
In response to Re: [HACKERS] shared memory based stat collector (was: Sharingrecord typmods between backends)  (Alvaro Herrera <alvherre@2ndquadrant.com>)
Responses Re: [HACKERS] shared memory based stat collector (was: Sharingrecord typmods between backends)  (Alvaro Herrera <alvherre@2ndquadrant.com>)
List pgsql-hackers
On Mon, Aug 14, 2017 at 1:12 PM, Alvaro Herrera
<alvherre@2ndquadrant.com> wrote:
> Yeah, the problem that lwlocks aren't released is because the launcher
> is not in a transaction at that point, so AbortCurrentTransaction()
> doesn't release locks like it normally would.  The simplest fix (in the
> attached 0001 patch) is to add a LWLockReleaseAll() call to the jmp
> block, though I wonder if there should be some other cleanup functions
> called from there, or whether perhaps it'd be a better strategy to have
> the launcher run in a transaction at all times.

Well, if you're going to do aborts outside of a transaction, just
adding an LWLockReleaseAll() isn't really sufficient.  You need to
look at something like CheckpointerMain() and figure out which of
those push-ups are needed here as well.  Probably at least   ConditionVariableCancelSleep(), pgstat_report_wait_end(),
AbortBufferIO(), and UnlockBuffers() -- quite possibly some of those
other AtEOXact calls as well.

> The other problem is that there's no attempt to handle a failed DSA
> creation/attachment.  The second patch just adds a PG_TRY block that
> sets a flag not to try the DSA calls again if the first one fails.  It
> throws a single ERROR line, then autovacuum continues without workitem
> support.

Yeah, and the other question -- which Thomas asked before you
originally committed originally, and which I just now asked again is
"Why in the world are you using DSA for this at all?".  There are
serious problems with that which both he and I have pointed out, and
you haven't explained why it's a good idea at any point, AFAICT.
Among those problems:

1. It doesn't work if dynamic_shared_memory_type=none.  That's OK for
an optional subsystem, but autovacuum is not very optional.

and

2. It allows unbounded bloat if there's no limit on the number of work
items and is pointless is there is since you could then just use the
main shared memory segment.

I really think you should respond to those concerns, not just push a
minimal fix.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company



pgsql-hackers by date:

Previous
From: Andres Freund
Date:
Subject: Re: [HACKERS] Orphaned files in base/[oid]
Next
From: Chris Travers
Date:
Subject: Re: [HACKERS] Orphaned files in base/[oid]