From: | Fujii Masao <masao(dot)fujii(at)gmail(dot)com> |
---|---|
To: | Michael Paquier <michael(dot)paquier(at)gmail(dot)com> |
Cc: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Fujii Masao <fujii(at)postgresql(dot)org>, pgsql-committers <pgsql-committers(at)postgresql(dot)org> |
Subject: | Re: pgsql: Fix connection leak in DROP SUBSCRIPTION command. |
Date: | 2017-03-08 14:46:29 |
Message-ID: | CAHGQGwFDmARHtMp-8P8Ta6B7p5mGhTT3c5D=TAJ5D-hD2cMCDA@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-committers |
On Wed, Feb 22, 2017 at 1:27 PM, Michael Paquier
<michael(dot)paquier(at)gmail(dot)com> wrote:
> On Wed, Feb 22, 2017 at 8:39 AM, Fujii Masao <masao(dot)fujii(at)gmail(dot)com> wrote:
>> On Wed, Feb 22, 2017 at 6:57 AM, Michael Paquier
>> <michael(dot)paquier(at)gmail(dot)com> wrote:
>>> On Wed, Feb 22, 2017 at 4:12 AM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>>>> Fujii Masao <fujii(at)postgresql(dot)org> writes:
>>>>> Fix connection leak in DROP SUBSCRIPTION command.
>>>>> Previously the command forgot to close the connection to the publisher
>>>>> when it failed to drop the replication slot.
>>>>
>>>> If there's a bug here, this seems like an extremely unreliable way of
>>>> fixing it. What if an error gets thrown before you reach that ereport?
>>>>
>>>> In other words, this coding is assuming that the walrcv_command()
>>>> subroutine cannot throw an error,
>>
>> Yes, but I agree that walrcv_command() may be changed in the future so that
>> an error is thrown and current coding is not reliable in that case.
>>
>>>> which I would consider dangerous
>>>> even if it were a fixed subroutine. If it's a hook that's doing
>>>> unknown stuff, that seems a completely untenable assumption. You
>>>> really need either to hook the cleanup action into normal error
>>>> recovery, or to use a PG_TRY block.
>>>
>>> To be honest, I have thought about using PG_ENSURE_ERROR_CLEANUP()
>>> when seeing the thread. If other ERROR messages are generated in the
>>> future that the current fix would be unreliable.
>>
>> What about the attached patch?
>
> Thanks for the patch. That looks good to me.
Petr and Michael,
Thanks for the review! Pushed.
Regards,
--
Fujii Masao
From | Date | Subject | |
---|---|---|---|
Next Message | Peter Eisentraut | 2017-03-08 15:10:04 | pgsql: pg_xlogdump: Remove extra newline in error message |
Previous Message | Fujii Masao | 2017-03-08 14:45:03 | pgsql: Fix connection leak in DROP SUBSCRIPTION command, take 2. |