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