From: Fujii Masao <masao.fujii@oss.nttdata.com>
To: Michael Paquier <michael@paquier.xyz>
To: Masahiko Sawada <masahiko.sawada@2ndquadrant.com>
Cc: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
Cc: sitnikov.vladimir@gmail.com, PostgreSQL-development <pgsql-hackers@postgresql.org>
Subject: Re: SIGSEGV from START_REPLICATION 0/XXXXXXX in XLogSendPhysical () at walsender.c:2762
Date: Tue, 2 Jun 2020 14:23:50 +0900
Message-ID: <b7b1fcaa-2532-a1e9-6533-d74954608003@oss.nttdata.com> (raw)
In-Reply-To: <20200602042456.GA3594@paquier.xyz>
References: <CAB=Je-HnFcwPUeLq5CXUqBnC7G7WeTi7DdGKbnFe-EizvnBJmw@mail.gmail.com>
<20200528.181139.1631626601933187948.horikyota.ntt@gmail.com>
<20200529072138.GG44192@paquier.xyz>
<20200529.175653.1322451159722162490.horikyota.ntt@gmail.com>
<CA+fd4k76X533vzD4S3_y8gyXNzpNyV8SQndKJL6jxLPqVAB9+A@mail.gmail.com>
<20200602042456.GA3594@paquier.xyz>
On 2020/06/02 13:24, Michael Paquier wrote:
> On Fri, May 29, 2020 at 06:09:06PM +0900, Masahiko Sawada wrote:
>> Yes. Conversely, if we start logical replication in a physical
>> replication connection (i.g. replication=true) we got an error before
>> staring replication:
>>
>> ERROR: logical decoding requires a database connection
>>
>> I think we can prevent that SEGV in a similar way.
>
> Still unconvinced as this restriction stands for logical decoding
> requiring a database connection but it is not necessarily true now as
> physical replication has less restrictions than a logical one.
Could you tell me what the benefit for supporting physical replication on
logical rep connection is? If it's only for "undocumented"
backward-compatibility, IMO it's better to reject such "tricky" set up.
But if there are some use cases for that, I'm ok to support that.
> Looking at the code, I think that there is some confusion with the
> fake WAL reader used as base reference in InitWalSender() where we
> assume that it could only be used in the context of a non-database WAL
> sender. However, this initialization happens when the WAL sender
> connection is initialized, and what I think this misses is that we
> should try to initialize a WAL reader when actually going through a
> START_REPLICATION command.
>
> I can note as well that StartLogicalReplication() moves in this sense
> by setting xlogreader to be the one from logical_decoding_ctx once the
> decoding context has been created.
>
> This results in the attached. The extra test from upthread to check
> that logical decoding is not allowed in a non-database WAL sender is a
> good idea, so I have kept it.
Yes. Also we should add the test to check if physical replication can work
fine even on logical rep connection?
Regards,
--
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: masao.fujii@oss.nttdata.com, michael@paquier.xyz, masahiko.sawada@2ndquadrant.com, horikyota.ntt@gmail.com
Subject: Re: SIGSEGV from START_REPLICATION 0/XXXXXXX in XLogSendPhysical () at walsender.c:2762
In-Reply-To: <b7b1fcaa-2532-a1e9-6533-d74954608003@oss.nttdata.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox