On Tue, Mar 22, 2016 at 1:53 AM, Jesper Pedersen <jesper.peder...@redhat.com> wrote: > On 03/18/2016 12:50 PM, Stas Kelvich wrote: >>> >>> On 11 Mar 2016, at 19:41, Jesper Pedersen <jesper.peder...@redhat.com> >>> wrote: >>> >> >> Thanks for review, Jesper. >> >>> Some comments: >>> >>> * The patch needs a rebase against the latest TwoPhaseFileHeader change >> >> >> Done. >> >>> * Rework the check.sh script into a TAP test case (src/test/recovery), as >>> suggested by Alvaro and Michael down thread >> >> >> Done. Originally I thought about reducing number of tests (11 right now), >> but now, after some debugging, I’m more convinced that it is better to >> include them all, as they are really testing different code paths. >> >>> * Add documentation for RecoverPreparedFromXLOG >> >> >> Done. > > > Thanks, Stas. > > I have gone over this version, and tested with --enable-tap-tests + make > check in src/test/recovery, which passes. > > Simon, do you want to move this entry to "Ready for Committer" and take the > 2REVIEWER section as part of that, or leave it in "Needs Review" and update > the thread ?
Looking at this patch.... +++ b/src/test/recovery/t/006_twophase.pl @@ -0,0 +1,226 @@ +# Checks for recovery_min_apply_delay +use strict; This description is wrong, this file has been copied from 005. +my $node_master = get_new_node("Candie"); +my $node_slave = get_new_node('Django'); Please let's use a node names that are more descriptive. +# Switch to synchronous replication +$node_master->append_conf('postgresql.conf', qq( +synchronous_standby_names = '*' +)); +$node_master->restart; Reloading would be fine. + /* During replay that lock isn't really necessary, but let's take it anyway */ + LWLockAcquire(TwoPhaseStateLock, LW_EXCLUSIVE); + for (i = 0; i < TwoPhaseState->numPrepXacts; i++) + { + gxact = TwoPhaseState->prepXacts[i]; + proc = &ProcGlobal->allProcs[gxact->pgprocno]; + pgxact = &ProcGlobal->allPgXact[gxact->pgprocno]; + + if (TransactionIdEquals(xid, pgxact->xid)) + { + gxact->locking_backend = MyBackendId; + MyLockedGxact = gxact; + break; + } + } + LWLockRelease(TwoPhaseStateLock); Not taking ProcArrayLock here? The comment at the top of XlogReadTwoPhaseData is incorrect. RecoverPreparedFromXLOG and RecoverPreparedFromFiles have a lot of code in common, having this duplication is not good, and you could simplify your patch. -- Michael -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers