Re: StandbyRecoverPreparedTransactions recovers subtrans links incorrectly

From: Simon Riggs <simon(at)2ndquadrant(dot)com>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: StandbyRecoverPreparedTransactions recovers subtrans links incorrectly
Date: 2017-04-25 06:47:38
Message-ID: CANP8+j+vvXmruL_i2buvdhMeVv5TQu0Hm2+C5N+kdVwHJuor8w@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On 23 April 2017 at 17:17, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Simon Riggs <simon(at)2ndquadrant(dot)com> writes:
>>> Also, when I fix that, it gets further but still crashes at the same
>>> Assert in SubTransSetParent. The proximate cause this time seems to be
>>> that RecoverPreparedTransactions's calculation of overwriteOK is wrong:
>>> it's computing that as "false", but in reality the subtrans link in
>>> question has already been set.
>
>> Not sure about that, investigating.
>
> As a quick hack, I just hotwired RecoverPreparedTransactions to set
> overwriteOK = true always, and with that and the SubTransSetParent
> argument-order fix, HEAD passes the recovery tests. Maybe we can
> be smarter than that, but this might be a good short-term fix to get
> the buildfarm green again.

So my thoughts are now that this is indeed the long term fix.

I can't think of why it would be necessary to call SubTransSetParent()
twice with two different values. A subtransaction's top level xid
never changes, so pg_subtrans should be either zero or the xid of the
parent and never anything else.

I can't see any reason now why overwriteOK should exist at all. I'm
guessing that the whole "overwriteOK" idea was an incorrect response
to xids appearing where they shouldn't have done because of the
mistake you just corrected. So I will now remove the parameter from
the call.

(I'll address the discussion of the impact of that bug in separate post).

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

Attachment Content-Type Size
2PC_remove_overwriteOK.v1.patch application/octet-stream 9.3 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Amit Langote 2017-04-25 07:07:17 Dropping a partitioned table takes too long
Previous Message Andres Freund 2017-04-25 06:19:41 Separation walsender & normal backends