Re: Something is broken about connection startup

On Mon, Nov 14, 2016 at 10:17 AM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>> I think the really important thing is that we don't leave xmin set
>> when the backend is idle.
>
> Agreed. I did some stats-gathering on this over the weekend, seeing how
> often the various situations occur during the core regression tests.
> It's not as big a problem as I first thought, because we hold a snapshot
> while doing planning, but the case does arise for various utility commands
> (particularly LOCK TABLE) so it's not negligible.
>
> What is looking like the best answer to me is to treat CatalogSnapshot
> as registered, but forcibly unregister it before going idle. We don't
> need to unregister it more often than that, because we'd hold a
> transaction snapshot of some description throughout any long-running
> command, and the CatalogSnapshot would be no older than that anyway.

Makes sense.

> "Before going idle" could be implemented with an InvalidateCatalogSnapshot
> call either in postgres.c's finish_xact_command(), or right in the main
> message-receipt loop. The latter seems like a wart, but it would cover
> cases like a client going to sleep between Bind and Execute phases of an
> extended query. On the other hand, I think we might be holding a
> transaction snapshot at that point anyway. It's at least arguable that
> we should be going through finish_xact_command() at any point where it's
> likely that we have a snapshot to release.

I assume you're going to back-patch this, and the consequences of
failing to reset it before going idle could easily be vastly worse
than the problem you're trying to fix. So I'd rather not make
assumptions like "the client will probably never sleep between Bind
and Execute". I bet that's actually false, and I certainly wouldn't
want to bet the farm on it being true.

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