Re: Support for REINDEX CONCURRENTLY

From: Andres Freund <andres(at)2ndquadrant(dot)com>
To: Michael Paquier <michael(dot)paquier(at)gmail(dot)com>
Cc: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Simon Riggs <simon(at)2ndquadrant(dot)com>, "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Support for REINDEX CONCURRENTLY
Date: 2013-02-12 12:13:24
Message-ID: 20130212121324.GB9120@alap2.anarazel.de
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi,

On 2013-02-07 16:45:57 +0900, Michael Paquier wrote:
> Please find attached a patch fixing 3 of the 4 problems reported before
> (the patch does not contain docs).
> 1) Removal of the quadratic dependency with list_append_unique_oid

Afaics you now simply lock objects multiple times, is that right?

> 2) Minimization of the wait phase for parent relations, this is done in a
> single transaction before phase 2

Unfortunately I don't think this did the trick. You currently have the
following:

+ /* Perform a wait on each session lock separate transaction */
+ StartTransactionCommand();
+ foreach(lc, lockTags)
+ {
+ LOCKTAG *localTag = (LOCKTAG *) lfirst(lc);
+ Assert(localTag && localTag->locktag_field2 != InvalidOid);
+ WaitForVirtualLocks(*localTag, ShareLock);
+ }
+ CommitTransactionCommand();

and

+void
+WaitForVirtualLocks(LOCKTAG heaplocktag, LOCKMODE lockmode)
+{
+ VirtualTransactionId *old_lockholders;
+
+ old_lockholders = GetLockConflicts(&heaplocktag, lockmode);
+
+ while (VirtualTransactionIdIsValid(*old_lockholders))
+ {
+ VirtualXactLock(*old_lockholders, true);
+ old_lockholders++;
+ }
+}

To get rid of the issue you need to batch all the GetLockConflicts calls
together before doing any of the VirtualXactLocks. Otherwise other
backends will produce new conflicts on relation n+1 while you wait for
relation n.

So it would need to be something like:

void
WaitForVirtualLocksList(List heaplocktags, LOCKMODE lockmode)
{
VirtualTransactionId **old_lockholders;
ListCell *lc;
int off = 0;
int i;

old_lockholders = palloc(sizeof(VirtualTransactionId *) *
list_length(heaplocktags));

/* collect transactions we need to wait on for all transactions */
foreach(lc, heaplocktags)
{
LOCKTAG *tag = lfirst(lc);
old_lockholders[off++] = GetLockConflicts(tag, lockmode);
}

/* wait on all transactions */
for (i = 0; i < off; i++)
{
VirtualTransactionId *lockholders = old_lockholders[i];

while (VirtualTransactionIdIsValid(lockholders[i]))
{
VirtualXactLock(lockholders[i], true);
lockholders++;
}
}
}

Makes sense?

Greetings,

Andres Freund

--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Peter Eisentraut 2013-02-12 12:14:06 pgsql: Add noreturn attributes to some error reporting functions
Previous Message Manlio Perillo 2013-02-12 12:00:57 Re: send Describe Portal message in PQsendPrepare