Postgres Pro
Downloads
Home   >   mailing lists

Thread: BUG #18118: bug report for COMMIT AND CHAIN feature

BUG #18118: bug report for COMMIT AND CHAIN feature

From
PG Bug reporting form
Date:
The following bug has been logged on the website:

Bug reference:      18118
Logged by:          Liu Xiang
Email address:      evan.lx@alibaba-inc.com
PostgreSQL version: 14.9
Operating system:   centos7
Description:

COMMIT(280a408b48d5ee42969f981bceb9e9426c3a344c) provides a way to support
chained commits. During recent development, I found that it left a flaw.

The defect can be reproduced as follows:

START TRANSACTION ISOLATION LEVEL SERIALIZABLE; COMMIT;
START TRANSACTION ISOLATION LEVEL REPEATABLE READ\; savepoint s\; COMMIT AND
CHAIN;
SHOW transaction_isolation; -- transaction is active at this point
COMMIT;

will get:
postgres=*# SHOW transaction_isolation; -- transaction is active at this
point
  transaction_isolation
-----------------------
  serializable
(1 row)

The reason for this bug is that in the case of
CommitTransactionCommand->TBLOCK_SUBCOMMIT, if there is a sub-transaction,
only RestoreTransactionCharacteristics will be performed instead of
SaveTransactionCharacteristics; this results in a global variable
save_XactIsoLevel remaining from the previous transaction being restored in
the transaction.

Thank you to the community for your continued help

Re: BUG #18118: bug report for COMMIT AND CHAIN feature

From
Laurenz Albe
Date:
On Wed, 2023-09-20 at 09:31 +0000, PG Bug reporting form wrote:
> PostgreSQL version: 14.9
> Operating system:   centos7
> Description:       
>
> COMMIT(280a408b48d5ee42969f981bceb9e9426c3a344c) provides a way to support
> chained commits. During recent development, I found that it left a flaw.
>
> The defect can be reproduced as follows:
>
> START TRANSACTION ISOLATION LEVEL SERIALIZABLE; COMMIT;
> START TRANSACTION ISOLATION LEVEL REPEATABLE READ\; savepoint s\; COMMIT AND
> CHAIN;
> SHOW transaction_isolation; -- transaction is active at this point
> COMMIT;
>
> will get:
> postgres=*# SHOW transaction_isolation; -- transaction is active at this
> point
>   transaction_isolation
> -----------------------
>   serializable
> (1 row)

I tried to reproduce that and failed, both with v14 and v16.

For me, "transaction_isolation" is "repeatable read".

Yours,
Laurenz Albe

回复：Re: BUG #18118: bug report for COMMIT AND CHAIN feature

From
"刘相(佑熙)"
Date:
This problem seems to have been solved in v15 and v16. Because of this patch (12d768e70497afc5a57acf73c251316997b5175a), the use of static variables has been cancelled. But this problem can obviously be reproduced on v14(and perhaps v12 v13). Try the following SQL again.

START TRANSACTION ISOLATION LEVEL SERIALIZABLE; COMMIT AND CHAIN; COMMIT ;
START TRANSACTION ISOLATION LEVEL SERIALIZABLE; COMMIT ;
START TRANSACTION ISOLATION LEVEL REPEATABLE READ; savepoint s; COMMIT AND CHAIN;
SHOW transaction_isolation; -- transaction is active at this point
COMMIT;
------------------------------------------------------------------
发件人：Laurenz Albe<laurenz.albe@cybertec.at>
日　期：2023年09月21日 14:41:20
收件人：刘相(佑熙)<evan.lx@alibaba-inc.com>; <pgsql-bugs@lists.postgresql.org>
主　题：Re: BUG #18118: bug report for COMMIT AND CHAIN feature

On Wed, 2023-09-20 at 09:31 +0000, PG Bug reporting form wrote:
> PostgreSQL version: 14.9
> Operating system:   centos7
> Description:        
> 
> COMMIT(280a408b48d5ee42969f981bceb9e9426c3a344c) provides a way to support
> chained commits. During recent development, I found that it left a flaw.
> 
> The defect can be reproduced as follows:
> 
> START TRANSACTION ISOLATION LEVEL SERIALIZABLE; COMMIT;
> START TRANSACTION ISOLATION LEVEL REPEATABLE READ\; savepoint s\; COMMIT AND
> CHAIN;
> SHOW transaction_isolation; -- transaction is active at this point
> COMMIT;
> 
> will get:
> postgres=*# SHOW transaction_isolation; -- transaction is active at this
> point
>   transaction_isolation
> -----------------------
>   serializable
> (1 row)

I tried to reproduce that and failed, both with v14 and v16.

For me, "transaction_isolation" is "repeatable read".

Yours,
Laurenz Albe

Re: 回复：Re: BUG #18118: bug report for COMMIT AND CHAIN feature

From
Tom Lane
Date:
"=?UTF-8?B?5YiY55u4KOS9keeGmSk=?=" <evan.lx@alibaba-inc.com> writes:
> This problem seems to have been solved in v15 and v16. Because of this patch
(12d768e70497afc5a57acf73c251316997b5175a),the use of static variables has been cancelled. But this problem can
obviouslybe reproduced on v14(and perhaps v12 v13). Try the following SQL again. 

Oooh ... I see the problem.  It's not really about the use of static
variables.  In the older branches, CommitTransactionCommand does

    if (s->chain)
        SaveTransactionCharacteristics();

....

                if (s->chain)
                    RestoreTransactionCharacteristics();

but if we're closing out any subtransactions then *those two checks
are inspecting different "s" structs*, and the "chain" flag has
only been set in the bottom of the state stack.

12d768e70 fixed the problem rather accidentally, because I removed the
initial "if (s->chain)" test to avoid a compiler warning.  I think we
can just do the same in the back branches, without having to address
the question of whether it'd be OK to break API for
SaveTransactionCharacteristics and RestoreTransactionCharacteristics.

            regards, tom lane