On 15/06/17 17:53, Peter Eisentraut wrote:
> On 6/14/17 18:35, Petr Jelinek wrote:
>> Attached fixes it (it was mostly about order of calls).
>
> So do I understand this right that the actual fix is just moving up the
> logicalrep_worker_stop() call in DropSubscription().
>
No the fix is heap_open before SearchSysCache().
>> I also split the
>> SetSubscriptionRelState into 2 separate interface while I was changing
>> it, because now that the update_only bool was added it has become quite
>> strange to have single interface for what is basically two separate
>> functions.
>
> makes sense
>
>> Other related problem is locking of subscriptions during operations on
>> them, especially AlterSubscription seems like it should lock the
>> subscription itself. I did that in 0002.
>
> More detail here please. AlterSubscription() does locking via
> heap_open(). This introduces a new locking method. What are the
> implications?
>
I don't think heap_open will be enough once we remove the
AccessExclusiveLock of the catalog in DropSubscription because
concurrent AlterSubscription might happily add tables to the
subscription that has been dropped if we don't lock it. But you made me
realize that even my patch is not enough because we then reread the
subscription info only from syscache without any kind of invalidation
attempt.
-- Petr Jelinek http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training &
Services
