From: Simon Riggs on

On Mon, 2009-09-21 at 13:50 +0300, Heikki Linnakangas wrote:

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

OK, I agree with that. Straightforward change. Thanks very much.

I marked the comment to indicate that the handling for GIST and GIN
indexes looked dubious to me also. I had the earlier "it is safe"
comments but that was before we looked at the kill prior tuples issue.

Re-reading code for GIN also, I note that there isn't any further work
because we don't kill prior tuples ever. Also, there is no special
handling of the GIN b-tree posting tree because VACUUM scans that in
logical sequence, rather than the physical sequence in nbtree.

--
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: Simon Riggs on

On Mon, 2009-09-21 at 13:50 +0300, Heikki Linnakangas wrote:
> 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.

Yes, clearly missing a check there. Those two operations should be
blocked at higher level, using PreventCommandDuringRecovery() and I
confess that I thought they already were. Doesn't crash because of the
other checks in place, but gives wrong error message.

Thanks for penetration testing the patch.

--
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: Simon Riggs on

On Mon, 2009-09-21 at 14:01 +0100, Simon Riggs wrote:
> On Mon, 2009-09-21 at 13:50 +0300, Heikki Linnakangas wrote:

> > 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.
>
> OK, I agree with that. Straightforward change. Thanks very much.
>
> I marked the comment to indicate that the handling for GIST and GIN
> indexes looked dubious to me also. I had the earlier "it is safe"
> comments but that was before we looked at the kill prior tuples issue.

ISTM I looked at this too quickly.

kill_prior_tuple is only ever set by these lines, after scan starts:

if (!scan->xactStartedInRecovery)
scan->kill_prior_tuple = scan->xs_hot_dead;

which is set in indexam.c, not within any particular am. So the coding,
as submitted, covers all index types, current and future.

AFAICS there is no bug, unless you have a test case or can explain
further?

Worth raising as a query because it forced me to re-check how GIST and
GIN work and am happy again now.

--
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: Robert Haas on
On Mon, Sep 21, 2009 at 9:01 AM, Simon Riggs <simon(a)2ndquadrant.com> wrote:
>
> On Mon, 2009-09-21 at 13:50 +0300, Heikki Linnakangas wrote:
>
>> The only bug I've found
>
> !

Yeah, wow.

....Robert

--
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: Bruce Momjian 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.

Wow, great! Simon has allowed us to pass a great milestone in Postgres
development.

--
Bruce Momjian <bruce(a)momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

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