Re: long-standing data loss bug in initial sync of logical replication

On Thu, 24 Apr 2025 at 14:39, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
>
> On Wed, Apr 23, 2025 at 10:28 PM Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com> wrote:
> >
> > >
> > > Fair enough. OTOH, we can leave the 13 branch considering following:
> > > (a) it is near EOL, (b) bug happens in rare cases (when the DDLs like
> > > ALTER PUBLICATION ... ADD TABLE ... or ALTER TYPE ... that don't take
> > > a strong lock on table happens concurrently to DMLs on the tables
> > > involved in the DDL.), and (c) the complete fix is invasive, even
> > > partial fix is not simple. I have a slight fear that if we make any
> > > mistake in fixing it partially (of course, we can't see any today), we
> > > may not even get a chance to fix it.
> > >
> > > Now, if the above convinces you or someone else not to push the
> > > partial fix in 13, then fine; otherwise, I'll push the 0001 to 13 day
> > > after tomorrow.
> >
> > I've considered the above points. I guess (b), particularly executing
> > ALTER PUBLICATION .. ADD TABLE while the target table is being
> > updated, might not be rare depending on systems. Given that this bug
> > causes a silent data-loss on the subscriber that is hard for users to
> > realize, it could ultimately depend on to what extent we can mitigate
> > the problem with only 0001 and there is a workaround when the problem
> > happens.
> >
> > Kuroda-san already shared[1] the analysis of what happens with and
> > without 0002 patch, but let me try with the example close to the
> > original data-loss problem[2]:
> >
> > Consider the following scenario:
> >
> > S1: CREATE TABLE d(data text not null);
> > S1: INSERT INTO d VALUES('d1');
> > S2: BEGIN;
> > S2: INSERT INTO d VALUES('d2');
> > S1: ALTER PUBLICATION pb ADD TABLE d;
> > S2: INSERT INTO d VALUES('d3');
> > S2: COMMIT
> > S2: INSERT INTO d VALUES('d4');
> > S1: INSERT INTO d VALUES('d5');
> >
> > Without 0001 and 0002 (i.e. as of today), the walsender fails to send
> > all changes to table 'd' until it invalidates its caches for some
> > reasons.
> >
> > With only 0001, the walsender sends 'd4' insertion or later.
> >
> > WIth both 0001 and 0002, the wansender sends 'd3' insertion or later.
> >
> > ISTM the difference between without both 0001 and 0002 and with 0001
> > is significant. So I think it's worth applying 0001 for v13.
> >
>
> Pushed to v13 as well, thanks for sharing the feedback.
>

In the commits, I saw that the filenames are misspelled for files
invalidation_distrubution.out and invalidation_distrubution.spec.
This is present in branches from REL_13 to HEAD. I have attached
patches to fix the same.

Thanks and Regards,
Shlok Kyal