From: Simon Riggs on 21 Sep 2009 09:01 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 21 Sep 2009 09:23 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 21 Sep 2009 15:52 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 21 Sep 2009 16:06 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 21 Sep 2009 22:07
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 |