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