Re: Allow single table VACUUM in transaction block

Hi Simon,

On Fri, Nov 4, 2022 at 10:15 AM Rahila Syed <rahilasyed90(at)gmail(dot)com> wrote:

> Hi Simon,
>
> On Thu, Nov 3, 2022 at 3:53 PM Simon Riggs <simon(dot)riggs(at)enterprisedb(dot)com>
> wrote:
>
>> On Tue, 1 Nov 2022 at 23:56, Simon Riggs <simon(dot)riggs(at)enterprisedb(dot)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