Re: FailedAssertion("pd_idx == pinfo->nparts", File: "execPartition.c", Line: 1689)

From: Andy Fan <zhihui(dot)fan1213(at)gmail(dot)com>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Robert Haas <robertmhaas(at)gmail(dot)com>, Amit Langote <amitlangote09(at)gmail(dot)com>, Justin Pryzby <pryzby(at)telsasoft(dot)com>, "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: FailedAssertion("pd_idx == pinfo->nparts", File: "execPartition.c", Line: 1689)
Date: 2020-08-07 02:26:21
Message-ID: CAKU4AWrYn449SNbwDHTSZZXJ1eGjwzrpCytTcDao7Wk4SvS1OA@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Fri, Aug 7, 2020 at 8:32 AM Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:

> Andy Fan <zhihui(dot)fan1213(at)gmail(dot)com> writes:
> > Attached is the v2 patch.
>
> Forgot to mention that I'd envisioned adding this as a src/test/modules/
> module; contrib/ is for things that we intend to expose to users, which
> I think this isn't.
>
> I played around with this and got the isolation test I'd experimented
> with yesterday to work with it. If you revert 7a980dfc6 then the
> attached patch will expose that bug. Interestingly, I had to add an
> explicit AcceptInvalidationMessages() call to reproduce the bug; so
> apparently we do none of those between planning and execution in the
> ordinary query code path. Arguably, that means we're testing a scenario
> somewhat different from what can happen in live databases, but I think
> it's OK. Amit's recipe for reproducing the bug works because there are
> other relation lock acquisitions (and hence AcceptInvalidationMessages
> calls) later in planning than where he asked us to wait. So this
> effectively tests a scenario where a very late A.I.M. call within the
> planner detects an inval event for some already-planned relation, and
> that seems like a valid-enough scenario.
>
> Anyway, attached find a reviewed version of your patch plus a test
> scenario contributed by me (I was too lazy to split it into two
> patches). Barring objections, I'll push this tomorrow or so.
>
> (BTW, I checked and found that this test does *not* expose the problems
> with Amit's original patch. Not sure if it's worth trying to finagle
> it so it does.)
>
> regards, tom lane
>
>
+ * delay_execution.c
+ * Test module to allow delay between parsing and execution of a query.

I am not sure if we need to limit the scope to "between parsing and
execution",
IMO, it can be used at any place where we have a hook so that
delay_execution
can inject the lock_unlock logic with a predefined lock id. Probably the
test writer
only wants one place blocked, then delay_execution.xxx_lock_id can be set
so
that only the given lock ID is considered. Just my 0.01 cents.

--
Best Regards
Andy Fan

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Masahiko Sawada 2020-08-07 02:40:23 Re: display offset along with block number in vacuum errors
Previous Message Peter Geoghegan 2020-08-07 02:00:46 Re: Should the nbtree page split REDO routine's locking work more like the locking on the primary?