Hi Simon, On Fri, Nov 4, 2022 at 10:15 AM Rahila Syed <rahilasye...@gmail.com> wrote:
> 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. > > Sorry, I just noticed that you have already mentioned some of these in the documentation as follows, so it seems it is already taken care of. + <command>VACUUM</command> cannot be executed inside a transaction block, + unless a single table is specified and <literal>FULL</literal> is not + specified. When executing inside a transaction block the vacuum scan can + hold back the xmin horizon and does not update the database datfrozenxid, + as a result this usage is not useful for database maintenance, but is provided + to allow vacuuming in special circumstances, such as temporary or private + work tables. Thank you, Rahila Syed
