Re: [HACKERS] Subscription code improvements

On Wed, Mar 7, 2018 at 11:13 PM, Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org>
wrote:
> 0002 looks like a good improvement to me. The existing routine is
> messy, and apparently it's so just to save one LockSharedObject plus
> cache lookup; IMO it's not worth it. Patched code looks simpler. If
> there are cases where having the combined behavior is useful, it's not
> clear what they are. (If I understand correctly, the reason is that a
> sync worker could try to insert-or-update the row after some other
> process deleted it [because of removing the table from subscription?]
> ... but that seems to work out *simpler* with the new code. So what's
> up?)
>

The function calling SetSubscriptionRelState with update_only=false (i.g.
going to do insert-or-update) is two function: CreateSubscription() and
AlterSubscription_refresh(). AFAICS these two function actually doesn't
need such insert-or-update functionality because it doesn't happen that a
backend process creates/alters the same name subscription which already
exists. Since CreateSubscirption() inserts a heap into the system catalog
one transaction ends up with the error of key already exists if two process
tries to create the same name subscription . Similarly for
AlterSubscription_refresh(), since we acquire the AccessExclusiveLock on
the subscription object before getting the new table list in the
publication the updating a existing entry doesn't happen. So this patch
changes SetsubscriptionRelState() with update_only=fasle to
AddSubscriptionRelState() and others to UpdateSubscriptionRelState().

Regards,

--
Masahiko Sawada
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center