Hi Simon, On Thu, Nov 3, 2022 at 3:53 PM Simon Riggs <simon.ri...@enterprisedb.com> wrote:
> On Tue, 1 Nov 2022 at 23:56, Simon Riggs <simon.ri...@enterprisedb.com> > wrote: > > > > I haven't checked the rest of the patch, but +1 for allowing VACUUM > FULL > > > within a user txn. > > > > My intention was to prevent that. I am certainly quite uneasy about > > changing anything related to CLUSTER/VF, since they are old, complex > > and bug prone. > > > > So for now, I will block VF, as was my original intent. > > > > I will be guided by what others think... so you may yet get your wish. > > > > > > > Maybe the error message needs to be qualified "...when multiple > > > relations are specified". > > > > > > ERROR: VACUUM cannot run inside a transaction block > > > > Hmm, that is standard wording based on the statement type, but I can > > set a CONTEXT message also. Will update accordingly. > > > > Thanks for your input. > > New version attached, as described. > > Other review comments and alternate opinions welcome. > > I applied and did some basic testing on the patch, it works as described. I would like to bring up a few points that I came across while looking into the vacuum code. 1. As a result of this change to allow VACUUM inside a user transaction, I think there is some chance of causing a block/delay of concurrent VACUUMs if a VACUUM is being run under a long running transaction. 2. Also, if a user runs VACUUM in a transaction, performance optimizations like PROC_IN_VACUUM won't work. 3. Also, if VACUUM happens towards the end of a long running transaction, the snapshot will be old and xmin horizon for vacuum would be somewhat old as compared to current lazy vacuum which acquires a new snapshot just before scanning the table. So, while I understand the need of the feature, I am wondering if there should be some mention of above caveats in documentation with the recommendation that VACUUM should be run outside a transaction, in general. Thank you, Rahila Syed
