From: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
---|---|
To: | Robert Haas <robertmhaas(at)gmail(dot)com> |
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 21:42:58 |
Message-ID: | 12785.1479159778@sss.pgh.pa.us |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
I wrote:
> Robert Haas <robertmhaas(at)gmail(dot)com> writes:
>> 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.
> 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.
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.
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.
Comments?
regards, tom lane
Attachment | Content-Type | Size |
---|---|---|
register-catalog-snapshot-1.patch | text/x-diff | 5.1 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Michael Paquier | 2016-11-14 21:43:07 | Re: [PATCH] Allow TAP tests to be run individually |
Previous Message | Greg Stark | 2016-11-14 20:43:12 | Re: Physical append-only tables |