Re: Global temporary tables

On 09.08.2019 8:34, Craig Ringer wrote:
> On Thu, 8 Aug 2019 at 15:03, Konstantin Knizhnik
> <k(dot)knizhnik(at)postgrespro(dot)ru <mailto:k(dot)knizhnik(at)postgrespro(dot)ru>> wrote:
>
>
>
> On 08.08.2019 5:40, Craig Ringer wrote:
>> On Tue, 6 Aug 2019 at 16:32, Konstantin Knizhnik
>> <k(dot)knizhnik(at)postgrespro(dot)ru <mailto:k(dot)knizhnik(at)postgrespro(dot)ru>> wrote:
>>
>> New version of the patch with several fixes is attached.
>> Many thanks to Roman Zharkov for testing.
>>
>>
>> FWIW I still don't understand your argument with regards to using
>> shared_buffers for temp tables having connection pooling
>> benefits. Are you assuming the presence of some other extension
>> in your extended  version of PostgreSQL ? In community PostgreSQL
>> a temp table's contents in one backend will not be visible in
>> another backend. So if your connection pooler in transaction
>> pooling mode runs txn 1 on backend 42 and it populates temp table
>> X, then the pooler runs the same app session's txn 2 on backend
>> 45, the contents of temp table X are not visible anymore.
>
> Certainly here I mean built-in connection pooler which is not
> currently present in Postgres,
> but it is part of PgPRO-EE and there is my patch for vanilla at
> commitfest:
> https://commitfest.postgresql.org/24/2067
>
>
> OK, that's what I assumed.
>
> You're trying to treat this change as if it's a given that the other
> functionality you want/propose is present in core or will be present
> in core. That's far from given. My suggestion is to split it up so
> that the parts can be reviewed and committed separately.
>
> In PgPRO-EE this problem was solved by binding session to backend.
> I.e. one backend can manage multiple sessions,
> but session can not migrate to another backend. The drawback of
> such solution is obvious: one long living transaction can block
> transactions of all other sessions scheduled to this backend.
> Possibility to migrate session to another backend is one of the
> obvious solutions of the problem. But the main show stopper for it
> is temporary tables.
> This is why  I consider moving temporary tables to shared buffers
> as very important step.
>
>
> I can see why it's important for your use case.
>
> I am not disagreeing.
>
> I am however strongly suggesting that your patch has two fairly
> distinct functional changes in it, and you should separate them out.
>
> * Introduce global temp tables, a new relkind that works like a temp
> table but doesn't require catalog changes. Uses per-backend
> relfilenode and cleanup like existing temp tables. You could extend
> the relmapper to handle the mapping of relation oid to per-backend
> relfilenode.
>
> * Associate global temp tables with session state and manage them in
> shared_buffers so they can work with the in-core connection pooler (if
> committed)
>
> Historically we've had a few efforts to get in-core connection pooling
> that haven't gone anywhere. Without your pooler patch the changes you
> make to use shared_buffers etc are going to be unhelpful at best, if
> not actively harmful to performance, and will add unnecessary
> complexity. So I think there's a logical series of patches here:
>
> * global temp table relkind and support for it
> * session state separation
> * connection pooling
> * pooler-friendly temp tables in shared_buffers
>
> Make sense?
>
> But even if we forget about built-in connection pooler, don't you
> think that possibility to use parallel query plans for temporary
> tables is itself strong enough motivation to access global temp
> table through shared buffers?
>
>
> I can see a way to share temp tables across parallel query backends
> being very useful for DW/OLAP workloads, yes. But I don't know if
> putting them in shared_buffers is the right answer for that. We have
> DSM/DSA, we have shm_mq, various options for making temp buffers
> share-able with parallel worker backends.
>
> I'm suggesting that you not tie the whole (very useful) global temp
> tables feature to this, but instead split it up into logical units
> that can be understood, reviewed and committed separately.
>
> I would gladly participate in review.

Ok, here it is: global_private_temp-1.patch
Also I have attached updated version of the global temp tables with
shared buffers - global_shared_temp-1.patch
It is certainly larger (~2k lines vs. 1.5k lines) because it is changing
BufferTag and related functions.
But I do not think that this different is so critical.
Still have a wish to kill two birds with one stone:)

> --

Konstantin Knizhnik
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company