Re: BUG #15290: Stuck Parallel Index Scan query

On Wed, Jul 25, 2018 at 2:08 PM, Andres Freund <andres@anarazel.de> wrote:
> On 2018-07-25 14:04:11 +1200, Thomas Munro wrote:
>> Ok, I see it:
>>
>>                         /* check for interrupts while we're not
>> holding any buffer lock */
>>                         CHECK_FOR_INTERRUPTS();
>>                         /* step right one page */
>>                         so->currPos.buf = _bt_getbuf(rel, blkno, BT_READ);
>>                         ...
>>                         /* nope, keep going */
>>                         if (scan->parallel_scan != NULL)
>>                         {
>>                                 status = _bt_parallel_seize(scan, &blkno);
>>
>> That leads to a condition variable wait, while we still hold that
>> buffer lock.  That prevents interrupts.  Oops.
>
> Heh, guessed right.  I kinda wonder if we shouldn't add a
> CHECK_FOR_INTERRUPTS_FOR_REALZ() that asserts if interrupts aren't
> held. There are plenty places where we rely on that being the case, for
> correctness.

Yeah, I was wondering something similar: perhaps WaitEventSetWait()
should assert that interrupts are not held, unless you explicitly told
it somehow that it's OK?  You couldn't do it unconditionally, because
we sometimes do that on purpose:

    HOLD_INTERRUPTS();
    WaitForParallelWorkersToExit(pcxt);
    RESUME_INTERRUPTS();

Here's a reproducer (adjust timeout to suit your machine):

drop table if exists t;
create table t as select generate_series(1, 1000000)::int i;
create index on t(i);
alter table t set (autovacuum_enabled = false);
delete from t; -- 100% bloat please

set max_parallel_workers_per_gather = 2;
set min_parallel_index_scan_size = 0;
set enable_seqscan = false;
set enable_bitmapscan = false;
set parallel_tuple_cost = 0;
set parallel_setup_cost = 0;

set statement_timeout = '100ms'; -- enough to fork, not enough to complete

explain analyze select count(*) from t;

-- 
Thomas Munro
http://www.enterprisedb.com