From: | Simon Riggs <simon(at)2ndQuadrant(dot)com> |
---|---|
To: | Heikki Linnakangas <heikki(dot)linnakangas(at)enterprisedb(dot)com> |
Cc: | pgsql-hackers(at)postgresql(dot)org |
Subject: | Re: Hot Standby, release candidate? |
Date: | 2009-12-14 12:22:57 |
Message-ID: | 1260793377.1955.602.camel@ebony |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Mon, 2009-12-14 at 11:07 +0000, Simon Riggs wrote:
> Thanks for the further review, much appreciated.
>
> On Mon, 2009-12-14 at 11:54 +0200, Heikki Linnakangas wrote:
> > Simon Riggs wrote:
> > > Enclose latest version of Hot Standby.
>
> > * LockAcquireExtended needs a function comment. Or at least something
> > explaining what report_memory_error does. And perhaps the argument
> > should be named "reportMemoryError" to be in line with the other arguments.
>
> OK
Done
> > * We tend to not add remarks about authors in code (comments in standby.c).
>
> OK
Done
> > * This optimization in GetConflictingVirtualXIDs():
> >
> > > + /*
> > > + * If we don't know the TransactionId that created the conflict, set
> > > + * it to latestCompletedXid which is the latest possible value.
> > > + */
> > > + if (!TransactionIdIsValid(limitXmin))
> > > + limitXmin = ShmemVariableCache->latestCompletedXid;
> > > +
> >
> > needs a lot more explanation. It took me a very long time to figure out
> > why using latest completed xid is safe.
>
> OK. Took me a long time as well.
Done
> > * Can you merge with CVS HEAD, please? There's some merge conflicts.
>
> Huh? I did. And tested patch against a CVS checkout before submitting.
> Can you explain further?
Still not sure what conflicts you see or where they might come from...
I am now unable to push these changes to the shared repo. What is
happening?
--
Simon Riggs www.2ndQuadrant.com
From | Date | Subject | |
---|---|---|---|
Next Message | KaiGai Kohei | 2009-12-14 12:30:49 | Re: [PATCH] ACE Framework - Database, Schema |
Previous Message | Bernd Helmle | 2009-12-14 12:19:27 | Re: [patch] executor and slru dtrace probes |