On 22/02/17 00:39, Fujii Masao wrote: > On Wed, Feb 22, 2017 at 6:57 AM, Michael Paquier > <michael.paqu...@gmail.com> wrote: >> On Wed, Feb 22, 2017 at 4:12 AM, Tom Lane <t...@sss.pgh.pa.us> wrote: >>> Fujii Masao <fu...@postgresql.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? >
Looks more or less like what we do in CREATE SUBSCRIPTION for this, so I guess it's okay. -- Petr Jelinek http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
