Re: Read-only access to temp tables for 2PC transactions

[Simon Riggs]

On Fri, 24 May 2019 at 18:09, Andres Freund  wrote:

> Hi,
>
> On 2019-05-24 19:37:15 +0300, Konstantin Knizhnik wrote:
> > From my point of view releasing all temporary table locks after
> preparing of
> > 2PC transaction is not technically possible:
> > assume that this transaction has  updated some tuples of temporary table
> - them
> > are not visible to other transactions until 2PC is committed,
> > but since lock is removed, other transactions can update the same tuple.
>
> I don't think tuple level actions are the problem? Those doesn't require
> table level locks to be held.
>
> Generally, I fail to see how locks themselves are the problem.


Agreed


> The
> problem are the catalog entries for the temp table, the relation forks,
> and the fact that a session basically couldn't drop (and if created in
> that transaction, use) etc the temp table after the PREPARE.
>

I don't see there is a problem here, but run out of time to explain more,
for a week.

-- 
Simon Riggshttp://www.2ndQuadrant.com/

PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Re: Read-only access to temp tables for 2PC transactions

[Andres Freund]

Hi,

On 2019-05-24 19:37:15 +0300, Konstantin Knizhnik wrote:
> From my point of view releasing all temporary table locks after preparing of
> 2PC transaction is not technically possible:
> assume that this transaction has  updated some tuples of temporary table - 
> them
> are not visible to other transactions until 2PC is committed,
> but since lock is removed, other transactions can update the same tuple.

I don't think tuple level actions are the problem? Those doesn't require
table level locks to be held.

Generally, I fail to see how locks themselves are the problem. The
problem are the catalog entries for the temp table, the relation forks,
and the fact that a session basically couldn't drop (and if created in
that transaction, use) etc the temp table after the PREPARE.

Greetings,

Andres Freund

Re: Read-only access to temp tables for 2PC transactions

[Konstantin Knizhnik]

On 24.05.2019 11:52, Simon Riggs wrote:
On Fri, 24 May 2019 at 01:39, Michael Paquier > wrote:


On Thu, May 23, 2019 at 08:54:59AM -0700, Andres Freund wrote:
> On 2019-05-23 12:36:09 +0100, Simon Riggs wrote:
>> The ONLY case where this matters is if someone does a PREPARE
and then
>> starts doing other work on the session. Which makes no sense in
the normal
>> workflow of a session. I'm sure there are tests that do that,
but those
>> tests are unrepresentative of sensible usage.
>
> That's extremely common.
>
> There's no way we can forbid using session after 2PC
unconditionally,
> it'd break most users of 2PC.

This does not break Postgres-XC or XL as their inner parts with a
COMMIT involving multiple write nodes issue a set of PREPARE
TRANSACTION followed by an immediate COMMIT PREPARED which are
transparent for the user, so the point of Simon looks sensible from
this angle. 



Maybe, but I am not discussing other products since they can be 
changed without discussion here.


Howewer, I much agree with Andres that it is very common
to have PREPARE and COMMIT PREPARED issued with different sessions.  I
am not much into the details of XA-compliant drivers, but I think that
having us lose this property would be the source of many complaints.


Yes, it is *very* common to have PREPARE and COMMIT PREPARED issued 
from different sessions. That is the main usage in a session pool and 
not the point I made.


There are two usage patterns, with a correlation between the way 2PC 
and temp tables work:


Transaction-mode session-pool: (Most common usage mode)
* No usage of session-level temp tables (because that wouldn't work)
* 2PC with PREPARE and COMMIT PREPARED on different sessions
* No reason at all to hold locks on temp table after PREPARE

Session-mode (Less frequent usage mode)
* Usage of session-level temp tables
* 2PC on same session only, i.e. no usage of session between PREPARE 
and COMMIT PREPARED (Simon's observation)
* No reason at all to hold locks on temp table after PREPARE (Simon's 
conclusion)


I'd like to hear from anyone that thinks my observation is incorrect 
and to explain their usage pattern so we can understand why they think 
they would execute further SQL between PREPARE and COMMIT PREPARED 
when using a single session, while at the same time using temp tables.


If there really is a usage pattern there we should take note of, then 
I suggest we introduce a parameter that allows temp table locks to be 
dropped at PREPARE, so that we can use 2PC and Temp Tables with ease, 
for those that want it.


--
Simon Riggs http://www.2ndQuadrant.com/ 
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services


From my point of view releasing all temporary table locks after 
preparing of 2PC transaction is not technically possible:
assume that this transaction has  updated some tuples of temporary table 
- them are not visible to other transactions until 2PC is committed,

but since lock is removed, other transactions can update the same tuple.

Prohibiting transaction to do anything else  except COMMIT/ROLLBACK 
PREPARED after preparing transaction seems to be too voluntaristic decision.
I do not think that "That's extremely common", but I almost sure that 
there are such cases.


The safe scenario is when temporary table is created and dropped inside 
transaction (table created with ON COMMIT DROP). But there is still one 
issue with this scenario: first creation of temporary table cause 
creation of
pg_temp namespace and it can not be undone. Another possible scenario is 
temporary table created outside transaction with ON COMMIT DELETE. In 
this case truncation of table on prepare will also release all locks.


Pure read-only access to temporary tables seems to be not so useful,  
because before reading something from temporary table, we have to write 
something to it. And if reading of temporary table is wrapped in 2PC,
then most likely writing to temporary table also has to be wrapped in 
2PC, which is not possible with the proposed solution.



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

Re: Read-only access to temp tables for 2PC transactions

[Simon Riggs]

On Fri, 24 May 2019 at 01:39, Michael Paquier  wrote:

> On Thu, May 23, 2019 at 08:54:59AM -0700, Andres Freund wrote:
> > On 2019-05-23 12:36:09 +0100, Simon Riggs wrote:
> >> The ONLY case where this matters is if someone does a PREPARE and then
> >> starts doing other work on the session. Which makes no sense in the
> normal
> >> workflow of a session. I'm sure there are tests that do that, but those
> >> tests are unrepresentative of sensible usage.
> >
> > That's extremely common.
> >
> > There's no way we can forbid using session after 2PC unconditionally,
> > it'd break most users of 2PC.
>
> This does not break Postgres-XC or XL as their inner parts with a
> COMMIT involving multiple write nodes issue a set of PREPARE
> TRANSACTION followed by an immediate COMMIT PREPARED which are
> transparent for the user, so the point of Simon looks sensible from
> this angle.


Maybe, but I am not discussing other products since they can be changed
without discussion here.


> Howewer, I much agree with Andres that it is very common
> to have PREPARE and COMMIT PREPARED issued with different sessions.  I
> am not much into the details of XA-compliant drivers, but I think that
> having us lose this property would be the source of many complaints.
>

Yes, it is *very* common to have PREPARE and COMMIT PREPARED issued from
different sessions. That is the main usage in a session pool and not the
point I made.

There are two usage patterns, with a correlation between the way 2PC and
temp tables work:

Transaction-mode session-pool: (Most common usage mode)
* No usage of session-level temp tables (because that wouldn't work)
* 2PC with PREPARE and COMMIT PREPARED on different sessions
* No reason at all to hold locks on temp table after PREPARE

Session-mode (Less frequent usage mode)
* Usage of session-level temp tables
* 2PC on same session only, i.e. no usage of session between PREPARE and
COMMIT PREPARED (Simon's observation)
* No reason at all to hold locks on temp table after PREPARE (Simon's
conclusion)

I'd like to hear from anyone that thinks my observation is incorrect and to
explain their usage pattern so we can understand why they think they would
execute further SQL between PREPARE and COMMIT PREPARED when using a single
session, while at the same time using temp tables.

If there really is a usage pattern there we should take note of, then I
suggest we introduce a parameter that allows temp table locks to be dropped
at PREPARE, so that we can use 2PC and Temp Tables with ease, for those
that want it.

-- 
Simon Riggshttp://www.2ndQuadrant.com/

PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Re: Read-only access to temp tables for 2PC transactions

[Simon Riggs]

On Thu, 23 May 2019 at 16:55, Andres Freund  wrote:

> Hi,
>
> On 2019-05-23 12:36:09 +0100, Simon Riggs wrote:
> > The ONLY case where this matters is if someone does a PREPARE and then
> > starts doing other work on the session. Which makes no sense in the
> normal
> > workflow of a session. I'm sure there are tests that do that, but those
> > tests are unrepresentative of sensible usage.
>
> That's extremely common.
>

Not at all.


> There's no way we can forbid using session after 2PC unconditionally,
> it'd break most users of 2PC.
>

Since we disagree, can you provide more information about this usage
pattern?

-- 
Simon Riggshttp://www.2ndQuadrant.com/

PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Re: Read-only access to temp tables for 2PC transactions

[Michael Paquier]

On Thu, May 23, 2019 at 08:54:59AM -0700, Andres Freund wrote:
> On 2019-05-23 12:36:09 +0100, Simon Riggs wrote:
>> The ONLY case where this matters is if someone does a PREPARE and then
>> starts doing other work on the session. Which makes no sense in the normal
>> workflow of a session. I'm sure there are tests that do that, but those
>> tests are unrepresentative of sensible usage.
> 
> That's extremely common.
> 
> There's no way we can forbid using session after 2PC unconditionally,
> it'd break most users of 2PC.

This does not break Postgres-XC or XL as their inner parts with a
COMMIT involving multiple write nodes issue a set of PREPARE
TRANSACTION followed by an immediate COMMIT PREPARED which are
transparent for the user, so the point of Simon looks sensible from
this angle.  Howewer, I much agree with Andres that it is very common
to have PREPARE and COMMIT PREPARED issued with different sessions.  I
am not much into the details of XA-compliant drivers, but I think that
having us lose this property would be the source of many complaints.
--
Michael


signature.asc
Description: PGP signature

Re: Read-only access to temp tables for 2PC transactions

[Andres Freund]

Hi,

On 2019-05-23 12:36:09 +0100, Simon Riggs wrote:
> The ONLY case where this matters is if someone does a PREPARE and then
> starts doing other work on the session. Which makes no sense in the normal
> workflow of a session. I'm sure there are tests that do that, but those
> tests are unrepresentative of sensible usage.

That's extremely common.

There's no way we can forbid using session after 2PC unconditionally,
it'd break most users of 2PC.

Greetings,

Andres Freund

Re: Read-only access to temp tables for 2PC transactions

[Simon Riggs]

On Tue, 14 May 2019 at 10:53, Stas Kelvich  wrote:


> One of the problems regarding the use of temporary tables in prepared
> transactions
> is that such transaction will hold locks for a temporary table after being
> prepared.
> That locks will prevent the backend from exiting since it will fail to
> acquire lock
> needed to delete temp table during exit. Also, re-acquiring such lock
> after server
> restart seems like an ill-defined operation.
>
...

> Any thoughts?
>

It occurs to me that there is no problem to solve here.

When we PREPARE, it is because we expect to COMMIT or ABORT soon afterwards.

If we are using an external transaction manager, the session is idle while
we wait for the manager to decide whether to commit or abort. Or the
session is disconnected or server is crashes. Either way, nothing happens
between PREPARE and resolution. So there is no need at all for locking of
temporary tables after the prepare.

The ONLY case where this matters is if someone does a PREPARE and then
starts doing other work on the session. Which makes no sense in the normal
workflow of a session. I'm sure there are tests that do that, but those
tests are unrepresentative of sensible usage.

If you were using session temporary tables while using a transaction mode
pool then you're already going to have problems, so that aspect is a
non-issue.

So I think we should ban this by definition. Say that we expect that you
won't do any work on the session until COMMIT/ABORT. That means we can then
drop locks on sesion temporary tables and drop on-commit temp tables when
we hit the prepare, not try and hold them for later.

A patch is needed to implement the above, but I think we can forget the
current patch as not needed.

-- 
Simon Riggshttp://www.2ndQuadrant.com/

PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Re: Read-only access to temp tables for 2PC transactions

[Stas Kelvich]

> On 14 May 2019, at 12:53, Stas Kelvich  wrote:
> 
> Hi,
> 
> That is an attempt number N+1 to relax checks for a temporary table access
> in a transaction that is going to be prepared.
> 

Konstantin Knizhnik made off-list review of this patch and spotted
few problems.

* Incorrect reasoning that ON COMMIT DELETE truncate mechanism
should be changed in order to allow preparing transactions with
read-only access to temp relations. It actually can be be leaved
as is. Things done in previous patch for ON COMMIT DELETE may be
a performance win, but not directly related to this topic so I've
deleted that part.

* Copy-paste error with check conditions in
relation_open/relation_try_open.

Fixed version attached.

--
Stas Kelvich
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company



2PC-ro-temprels-v2.patch
Description: Binary data

Read-only access to temp tables for 2PC transactions

[Stas Kelvich]

Hi,

That is an attempt number N+1 to relax checks for a temporary table access
in a transaction that is going to be prepared.

One of the problems regarding the use of temporary tables in prepared 
transactions
is that such transaction will hold locks for a temporary table after being 
prepared.
That locks will prevent the backend from exiting since it will fail to acquire 
lock
needed to delete temp table during exit. Also, re-acquiring such lock after 
server
restart seems like an ill-defined operation.

I tried to allow prepared transactions that opened a temporary relation only in
AccessShare mode and then neither transfer this lock to a dummy PGPROC nor 
include
it in a 'prepare' record. Such prepared transaction will not prevent the 
backend from
exiting and can be committed from other backend or after a restart.

However, that modification allows new DDL-related serialization anomaly: it 
will be
possible to prepare transaction which read table A; then drop A; then commit the
transaction. I not sure whether that is worse than not being able to access temp
relations or not. On the other hand, it is possible to drop AccessShare locks 
only for
temporary relation and don't change behavior for an ordinary table (in the 
attached
patch this is done for all tables).

Also, I slightly modified ON COMMIT DELETE code path. Right now all ON COMMIT 
DELETE
temp tables are linked in a static list and if transaction accessed any temp 
table
in any mode then during commit all tables from that list will be truncated. For 
a
given patch that means that even if a transaction only did read from a temp 
table it
anyway can access other temp tables with high lock mode during commit. I've 
added
hashtable that tracks higher-than-AccessShare action with a temp table during
current transaction, so during commit, only tables from that hash will be 
truncated.
That way ON COMMIT DELETE tables in the backend will not prevent read-only 
access to
some other table in a given backend.

Any thoughts?

--
Stas Kelvich
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company



2PC-ro-temprels.patch
Description: Binary data