Horton

Thread

Commits

  1. contrib/amcheck: Add heapam CHECK_FOR_INTERRUPTS().

  1. amcheck/verify_heapam doesn't check for interrupts

    Peter Geoghegan <pg@bowt.ie> — 2021-08-26T21:38:18Z

    I just noticed that the new heapam amcheck verification code can take
a very long time to respond to cancellations from pg_amcheck -- I saw
that it took over 2 minutes on a large database on my workstation.

It looks like we neglect to call CHECK_FOR_INTERRUPTS() anywhere
inside verify_heapam.c. Is there any reason for this? Can't we just
put a CHECK_FOR_INTERRUPTS() at the top of the outermost loop, inside
verify_heapam()?

Not sure if pg_amcheck itself is a factor here too -- didn't get that far.

Thanks
-- 
Peter Geoghegan

  2. Re: amcheck/verify_heapam doesn't check for interrupts

    Mark Dilger <mark.dilger@enterprisedb.com> — 2021-08-26T23:24:08Z

    
> On Aug 26, 2021, at 2:38 PM, Peter Geoghegan <pg@bowt.ie> wrote:
> 
> It looks like we neglect to call CHECK_FOR_INTERRUPTS() anywhere
> inside verify_heapam.c. Is there any reason for this?

Not any good one that I can see.

> Can't we just
> put a CHECK_FOR_INTERRUPTS() at the top of the outermost loop, inside
> verify_heapam()?

I expect we could.

> Not sure if pg_amcheck itself is a factor here too -- didn't get that far.

That runs an event loop in the client over multiple checks (heap and/or btree) running in backends, just as reindexdb and vacuumdb do over parallel reindexes and vacuums running in backends.  It should be just as safe to ctrl-c out of pg_amcheck as out of those two.  They all three use fe_utils/cancel.h's setup_cancel_handler(), so I would expect modifying verify_heapam would be enough.

—
Mark Dilger
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

  3. Re: amcheck/verify_heapam doesn't check for interrupts

    Peter Geoghegan <pg@bowt.ie> — 2021-08-26T23:39:18Z

    On Thu, Aug 26, 2021 at 4:24 PM Mark Dilger
<mark.dilger@enterprisedb.com> wrote:
> > On Aug 26, 2021, at 2:38 PM, Peter Geoghegan <pg@bowt.ie> wrote:
> > It looks like we neglect to call CHECK_FOR_INTERRUPTS() anywhere
> > inside verify_heapam.c. Is there any reason for this?
>
> Not any good one that I can see.

Seems that way. Want to post a patch?

> > Not sure if pg_amcheck itself is a factor here too -- didn't get that far.
>
> That runs an event loop in the client over multiple checks (heap and/or btree) running in backends, just as reindexdb and vacuumdb do over parallel reindexes and vacuums running in backends.  It should be just as safe to ctrl-c out of pg_amcheck as out of those two.  They all three use fe_utils/cancel.h's setup_cancel_handler(), so I would expect modifying verify_heapam would be enough.

Right. I checked that out myself, after sending my email from earlier.
We don't have any problems when pg_amcheck happens to be verifying a
B-Tree index -- verify_nbtree.c already has CHECK_FOR_INTERRUPTS() at
a few key points.

-- 
Peter Geoghegan

  4. Re: amcheck/verify_heapam doesn't check for interrupts

    Mark Dilger <mark.dilger@enterprisedb.com> — 2021-08-26T23:41:07Z

    
> On Aug 26, 2021, at 4:39 PM, Peter Geoghegan <pg@bowt.ie> wrote:
> 
>> Not any good one that I can see.
> 
> Seems that way. Want to post a patch?

Sure.  I just posted another unrelated patch for amcheck this morning, so it seems a good day for it :)

—
Mark Dilger
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

  5. Re: amcheck/verify_heapam doesn't check for interrupts

    Mark Dilger <mark.dilger@enterprisedb.com> — 2021-08-27T00:25:05Z

    
> On Aug 26, 2021, at 4:41 PM, Mark Dilger <mark.dilger@enterprisedb.com> wrote:
> 
>> Seems that way. Want to post a patch?
> 
> Sure.

  6. Re: amcheck/verify_heapam doesn't check for interrupts

    Peter Geoghegan <pg@bowt.ie> — 2021-08-27T01:42:35Z

    Patch committed.

Thanks!
-- 
Peter Geoghegan