References: <871rr3ohbo.fsf@ars-thinkpad>
 <CA+fd4k70BXLTm-N6q18LrL=GbKtwY3-2++UVFw05SvFTkZgTyQ@mail.gmail.com>
 <87v9oelex6.fsf@ars-thinkpad>
 <CA+fd4k6yyvS0UY5mvBMg9s__Lo05RYremSBPPNzkqPvKp4rsYg@mail.gmail.com>
 <87k1406sj7.fsf@ars-thinkpad> <87imjh7nky.fsf@ars-thinkpad>
 <CA+fd4k5qWBLTqomDxfeEeLR-yBf9_+EHxfFcpnP8hc7Bg15OEg@mail.gmail.com>
User-agent: mu4e 1.2.0; emacs 26.0.50
From: Arseny Sher <a.sher@postgrespro.ru>
To: Masahiko Sawada <masahiko.sawada@2ndquadrant.com>
Cc: PostgreSQL Hackers <pgsql-hackers@postgresql.org>,
 Alvaro Herrera <alvherre@alvh.no-ip.org>
Subject: Re: logical copy_replication_slot issues
In-reply-to: 
 <CA+fd4k5qWBLTqomDxfeEeLR-yBf9_+EHxfFcpnP8hc7Bg15OEg@mail.gmail.com>
Date: Mon, 09 Mar 2020 15:46:39 +0300
Message-ID: <87h7yx7l1s.fsf@ars-thinkpad>
MIME-Version: 1.0
Content-Type: text/plain
Precedence: bulk


Masahiko Sawada <masahiko.sawada@2ndquadrant.com> writes:

>     /*
> -    * Create logical decoding context, to build the initial snapshot.
> +    * Create logical decoding context to find start point or, if we don't
> +    * need it, to 1) bump slot's restart_lsn and xmin 2) check plugin sanity.
>      */
>
> Do we need to numbering that despite not referring them?

No, it just seemed clearer to me this way. I don't mind removing the
numbers if you feel this is better.

>     ctx = CreateInitDecodingContext(plugin, NIL,
> -                                   false,  /* do not build snapshot */
> +                                   false,  /* do not build data snapshot */
>                                     restart_lsn,
>                                     logical_read_local_xlog_page, NULL, NULL,
>                                     NULL);
> I'm not sure this change makes the comment better. Could you elaborate
> on the motivation of this change?

Well, DecodingContextFindStartpoint always builds a snapshot allowing
historical *catalog* lookups. This bool controls whether the snapshot
should additionally be suitable for looking at the actual data, this is
e.g. used by initial data sync in the native logical replication.


-- cheers, arseny