Re: Something is broken about connection startup

From: Robert Haas <robertmhaas(at)gmail(dot)com>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Something is broken about connection startup
Date: 2016-11-14 14:59:48
Message-ID: CA+TgmoZP63ddrFFuHiaO5gVM57a9hQE4RjhTLC6E8W_muNzU7g@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Fri, Nov 11, 2016 at 3:15 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> So this has pretty much been broken since we put in MVCC snapshots for
> catalog searches. The problem would be masked when inside a transaction
> that has already got a transaction snapshot, but whenever we don't have
> one already, our catalog lookups are basically unprotected against
> premature row removal. I don't see any reason to think that this is
> specific to connection startup.

Yeah. I was focused on the case where the backend is already in a
transaction, and obviously didn't think hard enough about the scenario
where that isn't the case.

> The basic problem here, therefore, is that SnapshotResetXmin isn't aware
> that GetCatalogSnapshot is keeping a possibly-unregistered snapshot in
> its hip pocket. That has to change. We could either treat the saved
> CatalogSnapshot as always being registered, or we could add some logic
> to force invalidating the CatalogSnapshot whenever we clear MyPgXact->xmin
> or advance it past that snapshot's xmin.
>
> Neither of those is very nice from a performance standpoint. With the
> first option we risk delaying global cleanup by holding back
> MyPgXact->xmin to protect a CatalogSnapshot we might not actually use
> anytime soon. With the second option we will likely end up doing many
> more GetSnapshotData calls than we do now, because in a transaction
> that hasn't yet set a regular snapshot, we will need to get a new
> CatalogSnapshot for every catalog access (since we'll go back to
> MyPgXact->xmin = 0 after every access). And the parser, in particular,
> tends to do a lot of catalog accesses before we ever get a regular
> transaction snapshot.
>
> Ideally, perhaps, we'd treat the saved CatalogSnapshot as registered
> but automatically kill it "every so often". I'm not sure how to drive
> that though.

I think the really important thing is that we don't leave xmin set
when the backend is idle. If we set MyProc->xmin during startup or in
some other out-of-transaction activity and never updated it until we
went idle, I think that wouldn't hurt much because we're not going to
spend tons of time in that state anyway. But if we start leaving xmin
set for idle backends, our users will be howling in agony.

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

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Peter Eisentraut 2016-11-14 15:02:13 Re: [PATCH] Allow TAP tests to be run individually
Previous Message Tom Lane 2016-11-14 14:57:37 Re: Partition-wise join for join between (declaratively) partitioned tables