Horton

Re: StandbyRecoverPreparedTransactions recovers subtrans links incorrectly

Tom Lane <tgl@sss.pgh.pa.us>

View thread View on postgr.es
From: Tom Lane <tgl@sss.pgh.pa.us>
To: Simon Riggs <simon@2ndquadrant.com>
Cc: Nikhil Sontakke <nikhils@2ndquadrant.com>, Andres Freund <andres@anarazel.de>, PostgreSQL-development <pgsql-hackers@postgresql.org>
Date: 2017-04-26T17:25:39Z
Lists: pgsql-hackers 
Simon Riggs <simon@2ndquadrant.com> writes:
> I've added code following Michael and Tom's comments to the previous
> patch. New patch attached.

Couple of minor suggestions:

* Rather than deleting the comment for SubTransSetParent entirely,
maybe make it say "It's possible that the parent was already recorded.
However, we should never be asked to change an already-set entry to
something else."

* In SubTransGetTopmostTransaction, maybe it'd be better to spell
"TransactionIdFollowsOrEquals" as "!TransactionIdPrecedes" to make
it look more like the test just above.  Matter of taste though.

* Less a matter of taste is that I think that should be just elog(ERROR);
there's no good reason to make it FATAL.

* Also, I think there should be a comment there, along the lines of
"check for reversed linkage to ensure this isn't an infinite loop".

			regards, tom lane

Commits

  1. Rework handling of subtransactions in 2PC recovery

  2. Fix order of arguments to SubTransSetParent().

  3. Speedup 2PC recovery by skipping two phase state files in normal path