Re: Global temporary tables

On 09.08.2019 8:34, Craig Ringer wrote:
> On Thu, 8 Aug 2019 at 15:03, Konstantin Knizhnik 
> <k.knizhnik@postgrespro.ru <mailto:k.knizhnik@postgrespro.ru>> wrote:
>
>
>
>     On 08.08.2019 5:40, Craig Ringer wrote:
>>     On Tue, 6 Aug 2019 at 16:32, Konstantin Knizhnik
>>     <k.knizhnik@postgrespro.ru <mailto:k.knizhnik@postgrespro.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