Re: Refreshing subscription relation state inside a transaction block

From: Petr Jelinek <petr(dot)jelinek(at)2ndquadrant(dot)com>
To: Peter Eisentraut <peter(dot)eisentraut(at)2ndquadrant(dot)com>, Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com>
Cc: PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Refreshing subscription relation state inside a transaction block
Date: 2017-06-15 14:49:35
Message-ID: 04c3c524-5f67-e992-af1a-1590497a2b50@2ndquadrant.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On 15/06/17 15:52, Peter Eisentraut wrote:
> On 6/15/17 02:41, Petr Jelinek wrote:
>> Hmm, forcibly stopping currently running table sync is not what was
>> intended, I'll have to look into it. We should not be forcibly stopping
>> anything except the main apply worker during drop subscription (and we
>> do that only because we can't drop the remote replication slot otherwise).
>
> The change being complained about was specifically to address the
> problem described in the commit message:
>
> Stop table sync workers when subscription relation entry is removed
>
> When a table sync worker is in waiting state and the subscription table
> entry is removed because of a concurrent subscription refresh, the
> worker could be left orphaned. To avoid that, explicitly stop the
> worker when the pg_subscription_rel entry is removed.
>
>
> Maybe that wasn't the best solution. Alternatively, the tablesync
> worker has to check itself whether the subscription relation entry has
> disappeared, or we need a post-commit check to remove orphaned workers.
>

Ah I missed that commit/discussion, otherwise I would have probably
complained. I don't like killing workers in the DDL command, as I said
the only reason we do it in DropSubscription is to free the slot for
dropping.

I think that either of the options you suggested now would be better.
We'll need that for stopping the tablesync which is in progress during
DropSubscription as well as those will currently still keep running. I
guess we could simply just register syscache callback, the only problem
with that is we'd need to AcceptInvalidationMessages regularly while we
do the COPY which is not exactly free so maybe killing at the end of
transaction would be better (both for refresh and drop)?

--
Petr Jelinek http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Tom Lane 2017-06-15 14:50:18 Re: intermittent failures in Cygwin from select_parallel tests
Previous Message Peter Eisentraut 2017-06-15 14:49:20 Re: logical replication: \dRp+ and "for all tables"