ALTER TABLE SET STATISTICS requires AccessExclusiveLock
in [PgSql]
|
Prev: pg_dump and join aliases (was Re: [BUGS] ERROR: cannot handle unplanned sub-select)
Next: patch: preload dictionary new version
From: Andres Freund on 18 Jul 2010 12:51 On Sunday 18 July 2010 18:02:26 Simon Riggs wrote: > On Sun, 2010-07-18 at 17:28 +0200, Andres Freund wrote: > > Unfortunately the same issue exists with constraint exclusion - and we > > can hardly disable that for serializable transactions... > > Then I think the fix is to look at the xmin values on all of the tables > used during planning and ensure that we only use constraint-based > optimisations in a serializable transaction when our top xmin is later > than the last DDL change (via its xmin). Why not just use a the normal snapshot at that point? Any older constraints should be just as valid for the tuples visible for the to-be-planned query. I also think that would lay groundwork for reducing lock-levels further in the future. Andres -- 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 18 Jul 2010 13:20 Andres Freund <andres(a)anarazel.de> writes: > On Sunday 18 July 2010 18:02:26 Simon Riggs wrote: >> Then I think the fix is to look at the xmin values on all of the tables >> used during planning and ensure that we only use constraint-based >> optimisations in a serializable transaction when our top xmin is later >> than the last DDL change (via its xmin). > Why not just use a the normal snapshot at that point? There isn't a "normal snapshot" that the planner should be relying on. It doesn't know what snap the resulting plan will be used with. I'm unconvinced that this is a problem worth worrying about, but if it is then Simon's probably got the right idea: check the xmin of a pg_constraint row before depending on it for planning. Compare the handling of indexes made with CREATE INDEX CONCURRENTLY. 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: Andres Freund on 18 Jul 2010 14:47 On Sunday 18 July 2010 19:20:25 Tom Lane wrote: > Andres Freund <andres(a)anarazel.de> writes: > > On Sunday 18 July 2010 18:02:26 Simon Riggs wrote: > >> Then I think the fix is to look at the xmin values on all of the tables > >> used during planning and ensure that we only use constraint-based > >> optimisations in a serializable transaction when our top xmin is later > >> than the last DDL change (via its xmin). > > > > Why not just use a the normal snapshot at that point? > There isn't a "normal snapshot" that the planner should be relying on. > It doesn't know what snap the resulting plan will be used with. Ok, I will write more stupid stuff in the next paragraph. Feel free to ignore. What I meant was to use * the transactions snapshot if we are in a transaction while planning * SnapshotNow otherwise (not sure if thats a situation really existing - I yet have no idea how such utitlity statements are handled snapshot-wise) The errors I described shouldn't matter for an already existing plan. Also the problem with a stale plan is already existing (only slightly aggravated due to the change) and should be handled via plan invalidation. Right? Phantasizing: If you continued with that you even could allow read only access to tables during ALTER TABLE et al. if the actual unlinking of the old filerelnode would get moved to the checkpoint or similar... > I'm unconvinced that this is a problem worth worrying about, but if it > is then Simon's probably got the right idea: check the xmin of a > pg_constraint row before depending on it for planning. Compare the > handling of indexes made with CREATE INDEX CONCURRENTLY. I am happy enough to write a docpatch for those issues and leave it there. Andres -- 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 18 Jul 2010 22:47 On Sun, Jul 18, 2010 at 1:20 PM, Tom Lane <tgl(a)sss.pgh.pa.us> wrote: > Andres Freund <andres(a)anarazel.de> writes: >> On Sunday 18 July 2010 18:02:26 Simon Riggs wrote: >>> Then I think the fix is to look at the xmin values on all of the tables >>> used during planning and ensure that we only use constraint-based >>> optimisations in a serializable transaction when our top xmin is later >>> than the last DDL change (via its xmin). > >> Why not just use a the normal snapshot at that point? > > There isn't a "normal snapshot" that the planner should be relying on. > It doesn't know what snap the resulting plan will be used with. > > I'm unconvinced that this is a problem worth worrying about, but if it > is then Simon's probably got the right idea: check the xmin of a > pg_constraint row before depending on it for planning. �Compare the > handling of indexes made with CREATE INDEX CONCURRENTLY. It generally seems like a Bad Thing to use one snapshot for planning and another snapshot for execution. For example, if one transaction (ostensibly serializable) runs a query twice in a row and in the mean time some other transaction redefines a function used by that query, the two runs will return different results, which is inconsistent with any serial order of execution of those transactions. But it seems that it's far from clear what to do about it, and it's not the job of this patch to fix it anyway. Regarding the actual patch, it looks mostly good. Questions: 1. Why in rewriteSupport.c are we adding a call to heap_inplace_update() in some situations? Doesn't seem like this is something we should need or want to be monkeying with. 2. Instead of AlterTableGreatestLockLevel(), how about AlterTableGetLockLevel()? Yeah, it's going to be the highest lock level required by any subcommand, but it seems mildly overspecified. I don't feel strongly about this one, though, if someone has a strong contrary opinion... -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise Postgres Company -- 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 19 Jul 2010 12:49
On Mon, Jul 19, 2010 at 2:46 AM, Simon Riggs <simon(a)2ndquadrant.com> wrote: > On Sun, 2010-07-18 at 22:47 -0400, Robert Haas wrote: >> �But it seems >> that it's far from clear what to do about it, and it's not the job of >> this patch to fix it anyway. > > Agreed. > >> Regarding the actual patch, it looks mostly good. �Questions: >> >> 1. Why in rewriteSupport.c are we adding a call to >> heap_inplace_update() in some situations? �Doesn't seem like this is >> something we should need or want to be monkeying with. > > Hmm, yes, that looks like a hangover. Will change. No others similar. > >> 2. Instead of AlterTableGreatestLockLevel(), how about >> AlterTableGetLockLevel()? �Yeah, it's going to be the highest lock >> level required by any subcommand, but it seems mildly overspecified. >> I don't feel strongly about this one, though, if someone has a strong >> contrary opinion... > > I felt it indicated the process it's using. Happy to change. Cool. I think with those two changes it's time to commit this. It's possible there's some case we've overlooked, but I think that we've been over this fairly thoroughly, so hopefully not. Anyway, we're doing this at the beginning of the 9.1 cycle rather than the end, so there's hopefully time for any lingering bugs to be found... -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise Postgres Company -- Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers |