Thanks Dilip for the patch. AFAIU, the fix looks good. One small comment:
In the error message we are still referring to the native protocol
version number. Shouldn't it be replaced with the greatest protocol
version number we support now (i.e. LOGICALREP_PROTO_MAX_VERSION_NUM)?
- if (data->protocol_version > LOGICALREP_PROTO_VERSION_NUM)
+ if (data->protocol_version > LOGICALREP_PROTO_MAX_VERSION_NUM)
ereport(ERROR,
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
errmsg("client sent proto_version=%d but we only
support protocol %d or lower",
data->protocol_version, LOGICALREP_PROTO_VERSION_NUM)));
Other than this, I don't have any comments.
On Mon, Sep 21, 2020 at 5:34 PM Dilip Kumar <dilipbal...@gmail.com> wrote:
>
> On Mon, Sep 21, 2020 at 4:15 PM Dilip Kumar <dilipbal...@gmail.com> wrote:
> >
> > On Mon, Sep 21, 2020 at 3:26 PM Ashutosh Sharma <ashu.coe...@gmail.com>
> > wrote:
> > >
> > > Hi All,
> > >
> > > Today, while exploring logical replication in PostgreSQL, I noticed
> > > that logical replication from PG version 13 and below to PG v14
> > > (development version) is not working. It has stopped working from the
> > > following git commit onwards:
> > >
> > > commit 464824323e57dc4b397e8b05854d779908b55304
> > > Author: Amit Kapila <akap...@postgresql.org>
> > > Date: Thu Sep 3 07:54:07 2020 +0530
> > >
> > > Add support for streaming to built-in logical replication.
> > >
> > > ...
> > > ...
> > >
> > > Here is the experiment that I performed to verify this:
> > >
> > > Publisher (PG-v12/13):
> > > ==================
> > > CREATE TABLE pg2pg (id serial PRIMARY KEY, whatever text);
> > >
> > > INSERT INTO pg2pg (whatever) SELECT 'str-' || i FROM generate_series(1,
> > > 5) i;
> > >
> > > SELECT * FROM pg2pg;
> > >
> > > CREATE PUBLICATION pg2pg_pub FOR TABLE pg2pg;
> > >
> > > Subscriber (PG-v14 HEAD or commit 46482432):
> > > =====================================
> > > CREATE TABLE pg2pg (id serial PRIMARY KEY, whatever text);
> > >
> > > CREATE SUBSCRIPTION pg2pg_sub CONNECTION 'host=127.0.0.1 port=5433
> > > dbname=postgres user=ashu' PUBLICATION pg2pg_pub;
> > >
> > > SELECT * FROM pg2pg;
> > >
> > > Above select query produces no result. When this experiment is
> > > performed below the mentioned git commit, it works fine.
> > >
> > > After spending some time looking into this issue, I observed that
> > > above git commit has bumped the logical replication protocol version.
> > > Due to this, the logical replication apply worker process is unable to
> > > do WAL streaming which causes it to terminate. Therefore, the data
> > > inserted in the publication table is not applied on the subscription
> > > table (i.e. no data replication happens)
> >
> > Seems like this commit, should have only set the
> > LOGICALREP_PROTO_STREAM_VERSION_NUM to 2 but the
> > LOGICALREP_PROTO_VERSION_NUM shouln't have been changed.
>
> I think if we don't increase the LOGICALREP_PROTO_VERSION_NUM, then
> streaming will not work in the latest version. So what I feel is that
> we can keep the LOGICALREP_PROTO_VERSION_NUM as 1 only add one more
> parameter say LOGICALREP_PROTO_MAX_VERSION_NUM. So now from the
> latest subscriber if streaming is enabled then we can send
> LOGICALREP_PROTO_STREAM_VERSION_NUM and LOGICALREP_PROTO_VERSION_NUM
> otherwise. And on publisher side we can check with the
> max_protocol_version and min_protocol version. I have attached a
> patch for the changes I have explained.
>
> --
> Regards,
> Dilip Kumar
> EnterpriseDB: http://www.enterprisedb.com
--
With Regards,
Ashutosh Sharma
EnterpriseDB:http://www.enterprisedb.com
