Re: Server Crashes if try to provide slot_name='none' at the time of creating subscription.

From: Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com>
To: Peter Eisentraut <peter(dot)eisentraut(at)2ndquadrant(dot)com>
Cc: tushar <tushar(dot)ahuja(at)enterprisedb(dot)com>, Kuntal Ghosh <kuntalghosh(dot)2007(at)gmail(dot)com>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Server Crashes if try to provide slot_name='none' at the time of creating subscription.
Date: 2017-05-18 02:50:42
Message-ID: CAD21AoAq8nAhoq7RVGg7ievvQOeM9LfHBdcr=n-0kcjqfjR+PQ@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Thu, May 18, 2017 at 10:33 AM, Peter Eisentraut
<peter(dot)eisentraut(at)2ndquadrant(dot)com> wrote:
> On 5/16/17 22:21, Masahiko Sawada wrote:
>> I think there are two bugs; pg_dump should dump slot_name = NONE
>> instead of '' and subscription should not be created if given slot
>> name is invalid. The validation check for replication slot name is
>> done when creating it actually but I think it's more safer to check
>> when CREATE SUBSCRIPTION. The bug related to setting slot_name = NONE
>> should be fixed by attached 001 patch, and 002 patch prevents to be
>> specified invalid replication slot name when CREATE SUBSCRIPTION and
>> ALTER SUBSCRIPTION SET.
>
> I have worked through these issues and came up with slightly different
> patches.
>
> The issue in pg_dump should have been checking PQgetisnull() before
> reading the slot name. That is now fixed.

Agreed.

>
> The issue with slot_name = NONE causing a crash was fixed by adding
> additional error checking. I did not change it so that slot_name = NONE
> would change the defaults of enabled and create_slot. I think it's
> confusing if options have dependencies like that. In any case, creating
> a subscription with slot_name = NONE is probably not useful anyway, so
> as long as it doesn't crash, we don't need to make it excessively
> user-friendly.

Also agreed.

> I don't think the subscription side should check the validity of the
> replication slot name. That is the responsibility of the publication
> side. The rules may well be different if you replicate between
> different versions or different build options. This is currently
> working correctly: If the publication side doesn't like the slot you
> specify, either because the name is invalid or the slot doesn't exist,
> you get an appropriate error message.

Yeah, now I understood. Agreed.

> Please check whether everything is working OK for you now. I think this
> open item is closed now.

I've checked these changes, everything is working fine. Thank you!

Regards,

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

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Tsunakawa, Takayuki 2017-05-18 03:07:33 Re: [bug fix] PG10: libpq doesn't connect to alternative hosts when some errors occur
Previous Message Peter Eisentraut 2017-05-18 02:29:06 Re: Improvement in log message of logical replication worker