From: | Thomas Munro <thomas(dot)munro(at)enterprisedb(dot)com> |
---|---|
To: | chris(dot)travers(at)adjust(dot)com |
Cc: | Pg Hackers <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: [PATCH] Fix for infinite signal loop in parallel scan |
Date: | 2018-09-14 11:16:05 |
Message-ID: | CAEepm=3HZxE+H5k9tY+z19=NkCjXuBhSS2QoCbO2NO-=BtirQA@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Sat, Sep 8, 2018 at 3:57 AM Chris Travers <chris(dot)travers(at)adjust(dot)com> wrote:
> Attached is the patch we are fully testing at Adjust.
Thanks!
> I have run make check on Linux and MacOS, and make check-world on Linux (check-world fails on MacOS on all versions and all branches due to ecpg failures).
FWIW it's entirely possible to get make check-world passing on a Mac.
Maybe post the problem you're seeing to a new thread?
> ...
> In the past it had been suggested we do PG_TRY(); and PG_CATCH(), but given that it is not consistent whether we can raise an error or whether we MUST raise an error, I don't see how this approach can work. As far as I can see, we MUST raise an error in the appropriate spot if and only if elevel is set to a sufficient level.
Yeah, your way looks a bit nicer than something involving PG_TRY().
> Is there any feedback on this approach before I add it to the next commitfest?
Please go ahead and add it. Being a bug fix, we'll commit it sooner
than the open commitfest anyway, but it's useful to have it in there.
+ if (errno == EINTR && elevel >= ERROR)
+ CHECK_FOR_INTERRUPTS();
I think we might as well just CHECK_FOR_INTERRUPTS() unconditionally.
In this branch elevel is always ERROR as you noted, and the code
around there is confusing enough already.
+ } while (rc == EINTR && !(ProcDiePending || QueryCancelPending));
There seems to be a precedent for checking QueryCancelPending directly
to break out of a loop in regcomp.c and syncrep.c. So this seems OK.
Hmm, I wonder if there should be an INTERRUPT_PENDING() macro that
hides those details, but allows you to break out of a loop and then do
some cleanup before CHECK_FOR_INTERRUPT().
--
Thomas Munro
http://www.enterprisedb.com
From | Date | Subject | |
---|---|---|---|
Next Message | Amit Khandekar | 2018-09-14 11:18:24 | Re: Query is over 2x slower with jit=on |
Previous Message | Amit Kapila | 2018-09-14 11:00:37 | Re: Problem while setting the fpw with SIGHUP |