Re: VACUUM (INTERRUPTIBLE)?

Hi,

On 2020-09-08 21:11:47 -0700, Andres Freund wrote:
> > Not yet sure about what a suitable way to test this for analyze would
> > be, unless we'd also accept VacuumDelay as a wait condition :(. I guess
> > one fairly easy way would be to include an expression index, and have
> > the expression index acquire an unavailable lock. Which'd allow to
> > switch to another session.
>
> But here I've not yet done anything. That just seems too ugly & fragile.

I found a better way after a bunch of tinkering. Having an
isolationtester session lock pg_class in SHARE mode works, because both
VACUUM and ANALYZE update the relation's row. I *think* this is
reliable, due to the report_multiple_error_messages() logic.

This version also adds setting of PROC_INTERRUPTIBLE for ANALYZE
(INTERUPTIBLE), which the previous version didn't yet contain. As
currently implemented this means that autovacuum started ANALYZEs are
interruptible, which they were not before. That's dead trivial to
change, but to me it makes sense to leave it this way. But I also see an
argument that it'd be better to do so separately?

> - It's a bit annoying that normal user backends just see a generic
> "ERROR: canceling statement due to user request". Do we need something
> better?

I think this is ok for this iteration, and instead at some point solve
this in a bit more generic manner. This isn't the only place where
carrying a bit more information about why and by whom a query is
cancelled would be very useful.

There's one XXX left in here, which is:

@@ -1340,8 +1338,12 @@ ProcSleep(LOCALLOCK *locallock, LockMethod lockMethodTable)
LWLockRelease(ProcArrayLock);

/* send the autovacuum worker Back to Old Kent Road */
+ /*
+ * FIXME: do we want to continue to identify autovacuum here?
+ * Could do so based on PROC_IS_AUTOVACUUM.
+ */
ereport(DEBUG1,
- (errmsg("sending cancel to blocking autovacuum PID %d",
+ (errmsg("sending cancel to blocking autovacuum|XXX PID %d",
pid),
errdetail_log("%s", logbuf.data)));

given that this is a DEBUG1 I'm inclined to just replace this with
"... blocking interruptible process with PID %d"
or such?

Comments?

Greetings,

Andres Freund