On Fri, Aug 7, 2020 at 8:32 AM Tom Lane <t...@sss.pgh.pa.us> wrote: > Andy Fan <zhihui.fan1...@gmail.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
