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-15 16:30:33
Message-ID: CA+TgmoY6Yci=2KnOe9jOzf1WW_ewxRu6VTdy8CGh5nGt=Kw48g@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Mon, Nov 14, 2016 at 4:42 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>> No, I'm not at all proposing to assume that. But I may be willing to
>> assume that "we don't hold a CatalogSnapshot between Bind and Execute
>> unless we're also holding a transaction snapshot". I need to do a bit
>> more research to see if that's actually true, though.
>
> Turns out it's not true.
>
> I still don't much like having the main loop in PostgresMain know about
> this hack, so I experimented with putting the InvalidateCatalogSnapshot()
> calls into places in postgres.c that were already dealing with transaction
> commit/abort or snapshot management. I ended up needing five such calls
> (as in the attached patch), which is just about equally ugly. So at this
> point I'm inclined to hold my nose and stick a call into step (1) of the
> main loop instead.

Seems like a good idea.

> Also, wherever we end up putting those calls, is it worth providing a
> variant invalidation function that only kills the catalog snapshot when
> it's the only one outstanding? (If it isn't, the transaction snapshot
> should be older, so there's no chance of advancing our xmin by killing
> it.) In principle this would save some catalog snapshot rebuilds for
> inside-a-transaction-block cases, but I'm not sure it's worth sweating
> that when we're doing client message exchange anyway.

I think that would be a fairly worthwhile thing to do.

> Lastly, I find myself disliking the separate CatalogSnapshotStale flag
> variable. The other special snapshots in snapmgr.c are managed by setting
> the pointer to NULL when it's not valid, so I wonder why CatalogSnapshot
> wasn't done that way. Since this patch is touching almost every use of
> that flag already, it wouldn't take much to switch it over.

I think I had some reason why I did it that way, but I don't think it
was anything important, so I don't object to you revising it.

--
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 Daniel Verite 2016-11-15 16:39:45 Re: proposal: psql \setfileref
Previous Message Tom Lane 2016-11-15 16:29:52 Re: Snapshot too old logging