Horton

Re: error context for vacuum to include block number

Justin Pryzby <pryzby@telsasoft.com>

View thread View on postgr.es
From: Justin Pryzby <pryzby@telsasoft.com>
To: Amit Kapila <amit.kapila16@gmail.com>
Cc: Masahiko Sawada <masahiko.sawada@2ndquadrant.com>, Alvaro Herrera <alvherre@2ndquadrant.com>, Andres Freund <andres@anarazel.de>, Michael Paquier <michael@paquier.xyz>, pgsql-hackers@postgresql.org
Date: 2020-03-27T19:04:29Z
Lists: pgsql-hackers 
On Fri, Mar 27, 2020 at 11:50:30AM +0530, Amit Kapila wrote:
> > > The crash scenario I'm trying to avoid would be like statement_timeout or other
> > > asynchronous event occurring between two non-atomic operations.
> > >
> > +if (errinfo->phase==VACUUM_ERRCB_PHASE_VACUUM_INDEX && errinfo->indname==NULL)
> > +{
> > +kill(getpid(), SIGINT);
> > +pg_sleep(1); // that's needed since signals are delivered asynchronously
> > +}
> > I'm not sure if those are possible outside of "induced" errors.  Maybe the
> > function is essentially atomic due to no CHECK_FOR_INTERRUPTS or similar?
> 
> Yes, this is exactly the point.  I think unless you have
> CHECK_FOR_INTERRUPTS in that function, the problems you are trying to
> think won't happen.

Hm, but I caused a crash *without* adding CHECK_FOR_INTERRUPTS, just
kill+sleep.  The kill() could come from running pg_cancel_backend().  And the
sleep() just encourages a context switch, which can happen at any time.  I'm
not convinced that the function couldn't be interrupted by a signal.

-- 
Justin

Commits

  1. Avoid calls to RelationGetRelationName() and RelationGetNamespace() in

  2. Introduce vacuum errcontext to display additional information.

  3. Fix mesurement of elapsed time during truncating heap in VACUUM.

  4. Allow vacuum command to process indexes in parallel.

  5. Refactor code dedicated to index vacuuming in vacuumlazy.c

  6. Remove duplicated progress reporting during heap scan of VACUUM