From: | Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> |
---|---|
To: | Tomas Vondra <tomas(dot)vondra(at)enterprisedb(dot)com> |
Cc: | Ashutosh Bapat <ashutosh(dot)bapat(dot)oss(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>, Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com>, Peter Eisentraut <peter(dot)eisentraut(at)enterprisedb(dot)com> |
Subject: | Re: logical decoding and replication of sequences, take 2 |
Date: | 2023-08-01 02:59:56 |
Message-ID: | CAA4eK1+KTL+hug2-Mcmcr_nsshwaLRjRa6arW79u5R8Jx49=uQ@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Mon, Jul 31, 2023 at 5:04 PM Tomas Vondra
<tomas(dot)vondra(at)enterprisedb(dot)com> wrote:
>
> On 7/31/23 11:25, Amit Kapila wrote:
> > On Sat, Jul 29, 2023 at 5:53 PM Tomas Vondra
> > <tomas(dot)vondra(at)enterprisedb(dot)com> wrote:
> >>
> >> On 7/28/23 14:44, Ashutosh Bapat wrote:
> >>> On Wed, Jul 26, 2023 at 8:48 PM Tomas Vondra
> >>> <tomas(dot)vondra(at)enterprisedb(dot)com> wrote:
> >>>>
> >>>> Anyway, I was thinking about this a bit more, and it seems it's not as
> >>>> difficult to use the page LSN to ensure sequences don't go backwards.
> >>>> The 0005 change does that, by:
> >>>>
> >>>> 1) adding pg_sequence_state, that returns both the sequence state and
> >>>> the page LSN
> >>>>
> >>>> 2) copy_sequence returns the page LSN
> >>>>
> >>>> 3) tablesync then sets this LSN as origin_startpos (which for tables is
> >>>> just the LSN of the replication slot)
> >>>>
> >>>> AFAICS this makes it work - we start decoding at the page LSN, so that
> >>>> we skip the increments that could lead to the sequence going backwards.
> >>>>
> >>>
> >>> I like this design very much. It makes things simpler than complex.
> >>> Thanks for doing this.
> >>>
> >>
> >> I agree it seems simpler. It'd be good to try testing / reviewing it a
> >> bit more, so that it doesn't misbehave in some way.
> >>
> >
> > Yeah, I also think this needs a review. This is a sort of new concept
> > where we don't use the LSN of the slot (for cases where copy returned
> > a larger value of LSN) or a full_snapshot created corresponding to the
> > sync slot by Walsender. For the case of the table, we build a full
> > snapshot because we use that for copying the table but why do we need
> > to build that for copying the sequence especially when we directly
> > copy it from the sequence relation without caring for any snapshot?
> >
>
> We need the slot to decode/apply changes during catchup. The main
> subscription may get ahead, and we need to ensure the WAL is not
> discarded or something like that. This applies even if the initial sync
> step does not use the slot/snapshot directly.
>
AFAIK, none of these needs a full_snapshot (see usage of
SnapBuild->building_full_snapshot). The full_snapshot tracks both
catalog and non-catalog xacts in the snapshot where we require to
track non-catalog ones because we want to copy the table using that
snapshot. It is relatively expensive to build a full snapshot and we
don't do that unless it is required. For the current usage of this
patch, I think using CRS_NOEXPORT_SNAPSHOT would be sufficient.
--
With Regards,
Amit Kapila.
From | Date | Subject | |
---|---|---|---|
Next Message | Michael Paquier | 2023-08-01 03:14:49 | Re: Support to define custom wait events for extensions |
Previous Message | Zhijie Hou (Fujitsu) | 2023-08-01 02:59:25 | RE: Simplify some logical replication worker type checking |