pg.ddx.io pgsql-hackers@postgresql.org mailing list archive
help / color / mirror / Atom feedFinishPreparedTransaction missing HOLD_INTERRUPTS section
3+ messages / 3 participants
[nested] [flat]
Commits from this thread:
2018-05-03 8f9be261f4 ee492e3ded d9b3bc5520 d3fc427f47 6bd659f19c 540e7a6e53 Add HOLD_INTERRUPTS section into FinishPreparedTransaction.
* FinishPreparedTransaction missing HOLD_INTERRUPTS section
@ 2018-04-27 21:36 Stas Kelvich <s.kelvich@postgrespro.ru>
0 siblings, 2 replies; 3+ messages in thread
From: Stas Kelvich @ 2018-04-27 21:36 UTC (permalink / raw)
To: pgsql-hackers
Hello.
It seems that during COMMIT PREPARED FinishPreparedTransaction() doesn't
hold interrupts around writing to wal and cleaning up ProcArray and GXact
entries. At least RemoveTwoPhaseFile (which is called in between) can print
a warning with ereport(), which, in turn will check for interrupts and
therefore can cancel backend or throw an error before GXact clean-up.
Other similar places like CommitTransaction and PrepareTransaction have
such hold interrupts sections.
--
Stas Kelvich
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company
Attachments:
0001-Add-HOLD_INTERRUPTS-section-into-FinishPreparedTrans.patch (application/octet-stream, 1.3K) download
From 8760cbda323b6e725db44e8c26d8d6a063d692a5 Mon Sep 17 00:00:00 2001
From: Stas Kelvich <stanconn@gmail.com>
Date: Fri, 27 Apr 2018 23:52:26 +0300
Subject: [PATCH] Add HOLD_INTERRUPTS section into FinishPreparedTransaction.
If an interrupt arrives in the middle of FinishPreparedTransaction
and any callback decide to call CHECK_FOR_INTERRUPTS (e.g.
RemoveTwoPhaseFile can write a warning with ereport, which checks for
interrupts) then it's possible to leave current GXact undeleted.
---
src/backend/access/transam/twophase.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/src/backend/access/transam/twophase.c b/src/backend/access/transam/twophase.c
index cdd8156ce4..a68769fb0d 100644
--- a/src/backend/access/transam/twophase.c
+++ b/src/backend/access/transam/twophase.c
@@ -1487,6 +1487,9 @@ FinishPreparedTransaction(const char *gid, bool isCommit)
/* compute latestXid among all children */
latestXid = TransactionIdLatest(xid, hdr->nsubxacts, children);
+ /* Prevent cancel/die interrupt while cleaning up */
+ HOLD_INTERRUPTS();
+
/*
* The order of operations here is critical: make the XLOG entry for
* commit or abort, then mark the transaction committed or aborted in
@@ -1579,6 +1582,7 @@ FinishPreparedTransaction(const char *gid, bool isCommit)
MyLockedGxact = NULL;
pfree(buf);
+ RESUME_INTERRUPTS();
}
/*
--
2.16.2
=
* Re: FinishPreparedTransaction missing HOLD_INTERRUPTS section
@ 2018-04-27 23:16 Michael Paquier <michael@paquier.xyz>
parent: Stas Kelvich <s.kelvich@postgrespro.ru>
1 sibling, 0 replies; 3+ messages in thread
From: Michael Paquier @ 2018-04-27 23:16 UTC (permalink / raw)
To: Stas Kelvich <s.kelvich@postgrespro.ru>; +Cc: pgsql-hackers
On Sat, Apr 28, 2018 at 12:36:16AM +0300, Stas Kelvich wrote:
> It seems that during COMMIT PREPARED FinishPreparedTransaction() doesn't
> hold interrupts around writing to wal and cleaning up ProcArray and GXact
> entries. At least RemoveTwoPhaseFile (which is called in between) can print
> a warning with ereport(), which, in turn will check for interrupts and
> therefore can cancel backend or throw an error before GXact clean-up.
>
> Other similar places like CommitTransaction and PrepareTransaction have
> such hold interrupts sections.
Good catch! The places you are suggesting look good to me as well.
That's something which should be back-patched as well.
--
Michael
Attachments:
signature.asc (application/pgp-signature, 832B) download
^ permalink raw reply [nested|flat] 3+ messages in thread
* Re: FinishPreparedTransaction missing HOLD_INTERRUPTS section
@ 2018-05-03 17:11 Teodor Sigaev <teodor@sigaev.ru>
parent: Stas Kelvich <s.kelvich@postgrespro.ru>
1 sibling, 0 replies; 3+ messages in thread
From: Teodor Sigaev @ 2018-05-03 17:11 UTC (permalink / raw)
To: Stas Kelvich <s.kelvich@postgrespro.ru>; pgsql-hackers
Thank you, pushed!
Stas Kelvich wrote:
> Hello.
>
> It seems that during COMMIT PREPARED FinishPreparedTransaction() doesn't
> hold interrupts around writing to wal and cleaning up ProcArray and GXact
> entries. At least RemoveTwoPhaseFile (which is called in between) can print
> a warning with ereport(), which, in turn will check for interrupts and
> therefore can cancel backend or throw an error before GXact clean-up.
>
> Other similar places like CommitTransaction and PrepareTransaction have
> such hold interrupts sections.
>
> --
> Stas Kelvich
> Postgres Professional: http://www.postgrespro.com
> The Russian Postgres Company
>
--
Teodor Sigaev E-mail: teodor@sigaev.ru
WWW: http://www.sigaev.ru/
^ permalink raw reply [nested|flat] 3+ messages in thread
end of thread, other threads:[~2018-05-03 17:11 UTC | newest]
Thread overview: 3+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2018-04-27 21:36 FinishPreparedTransaction missing HOLD_INTERRUPTS section Stas Kelvich <s.kelvich@postgrespro.ru>
2018-04-27 23:16 ` Michael Paquier <michael@paquier.xyz>
2018-05-03 17:11 ` Teodor Sigaev <teodor@sigaev.ru>
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox