Thread: CLUSTER, REINDEX, VACUUM in "read only" transaction?
So I was looking for other omissions in utility.c, and I noticed that check_xact_readonly() doesn't reject CLUSTER, REINDEX, or VACUUM. Now the notion of "read only" that we're trying to enforce is pretty weak (I think it's effectively "no writes to non-temp tables"). But I can't see that CLUSTER is a read-only operation even under the weakest definitions, and I'm not seeing the rationale for REINDEX or VACUUM here either. Comments? regards, tom lane
On Fri, Oct 10, 2008 at 09:41:39AM -0400, Tom Lane wrote: > So I was looking for other omissions in utility.c, and I noticed that > check_xact_readonly() doesn't reject CLUSTER, REINDEX, or VACUUM. > Now the notion of "read only" that we're trying to enforce is pretty > weak (I think it's effectively "no writes to non-temp tables"). > But I can't see that CLUSTER is a read-only operation even under the > weakest definitions, and I'm not seeing the rationale for REINDEX or > VACUUM here either. > > Comments? > > regards, tom lane > CLUSTER, REINDEX, and VACUUM are read-only to me because they do not change what I will call the actual working data in the database. Also, how could you address problems with index imbalance or heap organization? Regards, Ken
Kenneth Marshall <ktm@rice.edu> writes: > On Fri, Oct 10, 2008 at 09:41:39AM -0400, Tom Lane wrote: >> But I can't see that CLUSTER is a read-only operation even under the >> weakest definitions, and I'm not seeing the rationale for REINDEX or >> VACUUM here either. > CLUSTER, REINDEX, and VACUUM are read-only to me because they do not > change what I will call the actual working data in the database. Also, > how could you address problems with index imbalance or heap organization? Why would you expect to be able to address such things in a read-only transaction? regards, tom lane
On Fri, 2008-10-10 at 09:41 -0400, Tom Lane wrote: > So I was looking for other omissions in utility.c, and I noticed that > check_xact_readonly() doesn't reject CLUSTER, REINDEX, or VACUUM. > Now the notion of "read only" that we're trying to enforce is pretty > weak (I think it's effectively "no writes to non-temp tables"). > But I can't see that CLUSTER is a read-only operation even under the > weakest definitions, and I'm not seeing the rationale for REINDEX or > VACUUM here either. I think you should add a few more to the list. * LISTEN, UNLISTEN, NOTIFY * nextval() * ANALYZE * CHECKPOINT * GRANT, REVOKE, REASSIGN * DISCARD ALL should not run UnlistenAll These are all gaps I will have to plug for Hot Standby, and in fact wrote code this morning for some of those before I saw this post. (Yes, I went through every command). If you're gonna do it, do it soon please, so I know not to bother any further. -- Simon Riggs www.2ndQuadrant.comPostgreSQL Training, Services and Support
Tom Lane wrote: > So I was looking for other omissions in utility.c, and I noticed that > check_xact_readonly() doesn't reject CLUSTER, REINDEX, or VACUUM. > Now the notion of "read only" that we're trying to enforce is pretty > weak (I think it's effectively "no writes to non-temp tables"). > But I can't see that CLUSTER is a read-only operation even under the > weakest definitions, and I'm not seeing the rationale for REINDEX or > VACUUM here either. I think the way the SQL standard meant the read-only flag is that the transaction doesn't change the structure of or the data in the database as seen by the next guy. So all of these commands are OK, I think. A theoretical use case is that you should be able to do the maximum set of useful work in read-only mode on a Slony-I slave. No I haven't checked what Slony does with these three commands, so let me have it. :-) Other definitions might be OK, but I can't see one offhand that is based on the current behavior but disallows these three commands. "No disk writes" or "no big locks" is probably not what the SQL standard meant.
On Tue, 2008-10-14 at 19:18 +0300, Peter Eisentraut wrote: > Tom Lane wrote: > > So I was looking for other omissions in utility.c, and I noticed that > > check_xact_readonly() doesn't reject CLUSTER, REINDEX, or VACUUM. > > Now the notion of "read only" that we're trying to enforce is pretty > > weak (I think it's effectively "no writes to non-temp tables"). > > But I can't see that CLUSTER is a read-only operation even under the > > weakest definitions, and I'm not seeing the rationale for REINDEX or > > VACUUM here either. > > I think the way the SQL standard meant the read-only flag is that the > transaction doesn't change the structure of or the data in the database > as seen by the next guy. So all of these commands are OK, I think. > > A theoretical use case is that you should be able to do the maximum set > of useful work in read-only mode on a Slony-I slave. No I haven't > checked what Slony does with these three commands, so let me have it. :-) Well, read-only applies to queries on the Slony slave, not to other necessary work, which cannot be read only. In general, if one transaction is fully read-only I don't see why that should prevent other parts of the system from working normally. So I would say ban all the utilities mentioned from read-only transactions, and don't be influenced by what non-read only transactions do. -- Simon Riggs www.2ndQuadrant.comPostgreSQL Training, Services and Support