Обсуждение: 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