Re: Speedup twophase transactions - Mailing list pgsql-hackers

From Stas Kelvich
Subject Re: Speedup twophase transactions
Date
Msg-id E6684309-34AB-42E1-B7AB-EF7BFDA350C4@postgrespro.ru
Whole thread Raw
In response to Re: Speedup twophase transactions  (Michael Paquier <michael.paquier@gmail.com>)
Responses Re: Speedup twophase transactions  (Jesper Pedersen <jesper.pedersen@redhat.com>)
Re: Speedup twophase transactions  (Michael Paquier <michael.paquier@gmail.com>)
List pgsql-hackers
On Mar 29, 2016, at 6:04 PM, David Steele <david@pgmasters.net> wrote:

It looks like you should post a new patch or respond to Michael's comments.  Marked as "waiting on author".


Yep, here it is.

On Mar 22, 2016, at 4:20 PM, Michael Paquier <michael.paquier@gmail.com> wrote:

Looking at this patch….

Thanks.


+++ 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.

Yep, done.


+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.

Hm, it’s hard to create descriptive names because test changes master/slave roles for that nodes several times during test. It’s possible to call them “node1” and “node2” but I’m not sure that improves something. But anyway I’m not insisting on that particular names and will agree with any reasonable suggestion.


+# Switch to synchronous replication
+$node_master->append_conf('postgresql.conf', qq(
+synchronous_standby_names = '*'
+));
+$node_master->restart;
Reloading would be fine.

Good catch, done.


+   /* 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?

All accesses to 2pc dummies in ProcArray are covered with TwoPhaseStateLock, so I thick that’s safe. Also I’ve deleted comment above that block, probably it’s more confusing than descriptive.


The comment at the top of XlogReadTwoPhaseData is incorrect.

Yep, fixed.


RecoverPreparedFromXLOG and RecoverPreparedFromFiles have a lot of
code in common, having this duplication is not good, and you could
simplify your patch.

I reworked patch to avoid duplicated code between RecoverPreparedFromXLOG/RecoverPreparedFromFiles and also between FinishPreparedTransaction/XlogRedoFinishPrepared.


-- 
Stas Kelvich
Postgres Professional: http://www.postgrespro.com
Russian Postgres Company


Attachment

pgsql-hackers by date:

Previous
From: Jose Luis Tallon
Date:
Subject: Re: WIP: Access method extendability
Next
From: Teodor Sigaev
Date:
Subject: Re: WIP: Access method extendability