On Fri, Sep 25, 2020 at 7:21 AM Dilip Kumar <dilipbalaut@gmail.com> wrote:
>
> On Thu, Sep 24, 2020 at 5:31 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
> >
> > On Thu, Sep 24, 2020 at 5:11 PM Dilip Kumar <dilipbalaut@gmail.com> wrote:
> > >
> > > On Thu, Sep 24, 2020 at 4:45 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
> > > >
> > > >
> > > > Have you checked what will function walrcv_server_version() will
> > > > return? I was thinking that if we know that subscriber is connected to
> > > > Publisher version < 14 then we can send the right value.
> > >
> > > But, suppose we know the publisher version is < 14 and user has set
> > > streaming on while creating subscriber then still we will send the
> > > right version?
> > >
> >
> > Yeah we can send the version depending on which server we are talking to?
>
> Ok
>
> > > I think tablesync we are forming a query so we are
> > > forming as per the publisher's version. But here we are sending which
> > > protocol version we are expecting for the data transfer so I feel it
> > > should be LOGICALREP_PROTO_VERSION_NUM if we expect non-streaming
> > > transfer and LOGICALREP_PROTO_STREAM_VERSION_NUM if we expect the
> > > streaming transfer.
> > >
> >
> > I am not sure if this is the right strategy. See
> > libpqrcv_startstreaming, even if the user asked for streaming unless
> > the server supports it we don't send the streaming option to the user.
> > Another thing is this check will become more complicated if we need
> > another feature like decode prepared to send different version or even
> > if it is the same version, we might need additional checks. Why do you
> > want to send a streaming protocol version when we know the server
> > doesn't support it, lets behave similar to what we do in
> > libpqrcv_startstreaming.
>
> Okay, so even if the streaming is enabled we are sending the streaming
> on to the server versions which are >= 14 which make sense. But I
> still doubt that it is right thing to send the protocol version based
> on the server version. For example suppose in future PG20 we change
> the streaming protocol version to 3, that mean from PG14 to PG20 we
> may not support the streaming transmission but we still be able to
> support the normal transamission. Now if streaming is off then
> ideally we should send the LOGICALREP_PROTO_VERSION_NUM and that is
> still supporetd by the publisher but if we send the
> LOGICALREP_PROTO_STREAM_VERSION_NUM then it will error out because the
> streaming protocol changed in the latest subscriber and the same is
> not supported by the older publisher (14).
>
No that won't happen, we will send the version based on what the
server version supports (in this case it would be 2 when we are
talking to publisher 14). We can define a new macro or something like
that. I feel the protocol should depend on whether the server supports
it or not rather than based on some user specified option because it
will make our life easier if tomorrow we want to enable streaming by
default rather than based on option specified by the user. You can
consider it this way that how would you handle this if we wouldn't
have a user specified option (streaming) because that is generally the
way it should work.
--
With Regards,
Amit Kapila.
