Thread: checking for interrupts during heap insertion

Hi,

While talking to Amit Kapila this morning, he mentioned to me that
there seem to be no CHECK_FOR_INTERRUPTS() calls anywhere in
heap_multi_insert() or the functions it calls. Should there be?

By way of contrast, heapgetpage() has this:
   /*    * Be sure to check for interrupts at least once per page.  Checks at    * higher code levels won't be able to
stopa seqscan that encounters many    * pages' worth of consecutive dead tuples.    */   CHECK_FOR_INTERRUPTS();
 

In heap_multi_insert(), we first do heap_prepare_insert() on each
tuple, which may involve dirtying many pages, since it handles TOAST.
Then, we loop over the tuples themselves and dirty a bunch more pages.
All of that will normally happen pretty quickly, but if the I/O
subsystem is very slow for some reason, such as due to heavy system
load, then it might take quite a long time.  I'm thinking we might
want a CHECK_FOR_INTERRUPTS() in the following two places:

1. Inside toast_save_datum, at the top of the loop that starts with
"while (data_todo > 0)".
2. Inside heap_multi_insert, at the top of the loop that starts with
"while (ndone < ntuples)".

Thoughts?

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

On 06/23/2014 08:07 PM, Robert Haas wrote:
> While talking to Amit Kapila this morning, he mentioned to me that
> there seem to be no CHECK_FOR_INTERRUPTS() calls anywhere in
> heap_multi_insert() or the functions it calls. Should there be?

Haven't heard any complaints, but I guess..

> By way of contrast, heapgetpage() has this:
>
>      /*
>       * Be sure to check for interrupts at least once per page.  Checks at
>       * higher code levels won't be able to stop a seqscan that encounters many
>       * pages' worth of consecutive dead tuples.
>       */
>      CHECK_FOR_INTERRUPTS();
>
> In heap_multi_insert(), we first do heap_prepare_insert() on each
> tuple, which may involve dirtying many pages, since it handles TOAST.
> Then, we loop over the tuples themselves and dirty a bunch more pages.
> All of that will normally happen pretty quickly, but if the I/O
> subsystem is very slow for some reason, such as due to heavy system
> load, then it might take quite a long time.  I'm thinking we might
> want a CHECK_FOR_INTERRUPTS() in the following two places:
>
> 1. Inside toast_save_datum, at the top of the loop that starts with
> "while (data_todo > 0)".
> 2. Inside heap_multi_insert, at the top of the loop that starts with
> "while (ndone < ntuples)".

Seems reasonable.

- Heikki

On Mon, Jun 23, 2014 at 3:30 PM, Heikki Linnakangas
<hlinnakangas@vmware.com> wrote:
>> load, then it might take quite a long time.  I'm thinking we might
>> want a CHECK_FOR_INTERRUPTS() in the following two places:
>>
>> 1. Inside toast_save_datum, at the top of the loop that starts with
>> "while (data_todo > 0)".
>> 2. Inside heap_multi_insert, at the top of the loop that starts with
>> "while (ndone < ntuples)".
>
> Seems reasonable.

OK, done.

I don't have time to pursue this at the moment, but while looking at
this, I was also wondering if toast_save_datum() ought to be rewritten
to use heap_multi_insert().  I seem to recall that you found that the
multi-insert optimization mostly benefited narrow tables, so I'm not
sure whether it would work out to a win, but maybe it'd be worth
investigating.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company