From: Simon Riggs on

On Fri, 2009-09-18 at 07:23 -0700, Jeff Janes wrote:
> On Tue, Sep 15, 2009 at 2:41 PM, Simon Riggs <simon(a)2ndquadrant.com>
> wrote:
>
> OK, here is the latest version of the Hot Standby patchset.
> This is
> about version 30+ by now, but we should regard this as 0.2.1
> Patch against CVS HEAD (now): clean apply, compile, no known
> bugs.

> Is there a reason that you remove the WAL_DEBUG shown below?

WAL_DEBUG is not removed by the patch, though that section of code is
removed, as you observe. I recall an earlier bug report by
me/conversation on hackers about how that section of code was
irrecoverably broken, since it's calling an rmgr routine while not in
recovery and also assuming the data is fully accessible at that point,
which it is not.

--
Simon Riggs www.2ndQuadrant.com


--
Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

From: Tom Lane on
Simon Riggs <simon(a)2ndQuadrant.com> writes:
> On Fri, 2009-09-18 at 07:23 -0700, Jeff Janes wrote:
>> Is there a reason that you remove the WAL_DEBUG shown below?

> WAL_DEBUG is not removed by the patch, though that section of code is
> removed, as you observe. I recall an earlier bug report by
> me/conversation on hackers about how that section of code was
> irrecoverably broken, since it's calling an rmgr routine while not in
> recovery and also assuming the data is fully accessible at that point,
> which it is not.

Wouldn't it be sufficient to remove the rm_desc() call? I agree
that that's broken, but the rest doesn't seem to be.

regards, tom lane

--
Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

From: Simon Riggs on

On Fri, 2009-09-18 at 11:14 -0400, Tom Lane wrote:
> Simon Riggs <simon(a)2ndQuadrant.com> writes:
> > On Fri, 2009-09-18 at 07:23 -0700, Jeff Janes wrote:
> >> Is there a reason that you remove the WAL_DEBUG shown below?
>
> > WAL_DEBUG is not removed by the patch, though that section of code is
> > removed, as you observe. I recall an earlier bug report by
> > me/conversation on hackers about how that section of code was
> > irrecoverably broken, since it's calling an rmgr routine while not in
> > recovery and also assuming the data is fully accessible at that point,
> > which it is not.
>
> Wouldn't it be sufficient to remove the rm_desc() call? I agree
> that that's broken, but the rest doesn't seem to be.

That would make sense also. Previous action just because that was
earlier consensus. Will change.

--
Simon Riggs www.2ndQuadrant.com


--
Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

From: Marcos Luis Ortiz Valmaseda on
I want to help on this area, but I need a mentor for this.
For example, Heikki will be a excellent mentor for me.

Following the theme, I think that we have to wide all questions for the process of the acceptance of a patch on the same way that you Simon.

We could write new requirements with all these ideas. Don´t you think?

Regards

"The hurry is enemy of the success: for that reason.......Be patient"

Ing. Marcos L. Ortiz Valmaseda
Línea Soporte y Despliegue
Centro de Tecnologías de Almacenamiento y Análisis de Datos (CENTALAD)

Linux User # 418229
PostgreSQL User
http://www.postgresql.org
http://www.planetpostgresql.org/
http://www.postgresql-es.org/


----- Mensaje original -----
De: "Simon Riggs" <simon(a)2ndQuadrant.com>
Para: "Robert Haas" <robertmhaas(a)gmail.com>
CC: "Heikki Linnakangas" <heikki.linnakangas(a)enterprisedb.com>, "Josh Berkus" <josh(a)agliodbs.com>, pgsql-hackers(a)postgresql.org
Enviados: Jueves, 17 de Septiembre 2009 20:53:24 GMT -10:00 Hawai
Asunto: Re: [HACKERS] Hot Standby 0.2.1


On Thu, 2009-09-17 at 19:01 -0400, Robert Haas wrote:
> >
> > I'm going to put the index-only scans aside for now to focus on hot
> > standby and streaming replication. Both are big patches, so there's
> > plenty of work in those two alone, and not only for me.
>
> What is the best way to attack this? Should I keep reviewing
> index-only scans so that you have feedback for when you get back to
> it, or should I move on to something else? If something else, does it
> make more sense for me to look at HS since I did a bit of work with a
> previous version, or would it be better for me to just pick one of the
> other patches from the CommitFest and work on that?
>
> Also, stepping back from me personally, should we try to assign some
> additional reviewers to these patches? Is there some way we can
> divide up review tasks among multiple people so that we're not
> repeating each others work?
>
> Thoughts appreciated, from Heikki, Simon, or others.

I think this is a great opportunity to widen the pool of people
contributing to reviews.

I suggest the creation of a second group of people, performing
round-robin testing of patches. These people would be able to verify
* documentation matches implemented features (does it do what it says on
the tin?)
* usability of explicit features (do the features work well?)
* usability gap of unimplemented features (what else do we need?)
* are there any bugs?

These questions are often quickly answered for smaller patches, but HS's
scope mean that such a task properly executed could take a full week, if
not longer.

Second group of people are just as skilled Postgres people as reviewers,
in some cases more so, apart from they have less detailed knowledge of
the codebase. We have many such people and it would be good to encourage
them to perform thorough reviews rather than "tire kicking".

I'm not sure that Heikki needs additional reviewers. He now has
significant knowledge of the patch and is good at focusing on key
aspects of the internals. Other code reviewers are welcome, of course.

--
Simon Riggs www.2ndQuadrant.com


--
Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

--
Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

From: Heikki Linnakangas on
Simon Riggs wrote:
> OK, here is the latest version of the Hot Standby patchset. This is
> about version 30+ by now, but we should regard this as 0.2.1
> Patch against CVS HEAD (now): clean apply, compile, no known bugs.

Thanks! Attached is some minor comment and fixes, and some dead code
removal. Also available in my git repository, branch 'hs-riggs'.

The documentation talks about setting and checking
default_transaction_read_only, but I think it doesn't say anything about
transaction_read_only, which I find odd. This in particular:

> Users will be able to tell whether their session is read-only by
> + issuing SHOW default_transaction_read_only

seems misleading, as you might have default_transaction_read_only=on,
but still be able to do "SET transaction_read_only", so the *session*
isn't necessarily read-only.

The only bug I've found is this that we seem to be missing conflict
resolution for GiST index tuples deleted by the kill_prior_tuples
mechanism. Unless I'm missing something, we need similar handling there
that we have in b-tree.

--
Heikki Linnakangas
EnterpriseDB http://www.enterprisedb.com