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

Shlok Kyal <shlok.kyal.oss@gmail.com>

From: Shlok Kyal <shlok.kyal.oss@gmail.com>
To: Amit Kapila <amit.kapila16@gmail.com>
Cc: vignesh C <vignesh21@gmail.com>, Masahiko Sawada <sawada.mshk@gmail.com>, Nitin Motiani <nitinmotiani@google.com>, Tomas Vondra <tomas.vondra@enterprisedb.com>, Andres Freund <andres@anarazel.de>, PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Date: 2024-08-30T09:35:48Z
Lists: pgsql-hackers

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →
  1. Fix typo in test file name added in commit 4909b38af0.

  2. Fix data loss in logical replication.

  3. Avoid invalidating all RelationSyncCache entries on publication rename.

  4. Remove obsolete RECHECK keyword completely

  5. Backport BackgroundPsql perl test module

Attachments

> BTW, we should do some performance testing by having a mix of DML and
> DDLs to see the performance impact of this patch.
>
> [1] - https://www.postgresql.org/message-id/CAD21AoAenVqiMjpN-PvGHL1N9DWnHSq673bfgr6phmBUzx=kLQ@mail.gmail.com
>

I did some performance testing and I found some performance impact for
the following case:

1. Created a publisher, subscriber set up on a single table, say 'tab_conc1';
2. Created a second publisher, subscriber set on a single table say 'tp';
3. Created 'tcount' no. of tables. These tables are not part of any publication.
4. There are two sessions running in parallel, let's say S1 and S2.
5. Begin a transaction in S1.
6. Now in a loop (this loop runs 100 times):
     S1: Insert a row in table 'tab_conc1'
     S1: Insert a row in all 'tcount' tables.
     S2: BEGIN;  Alter publication for 2nd publication; COMMIT;
        The current logic in the patch will call the function
'rel_sync_cache_publication_cb' during invalidation. This will
invalidate the cache for all the tables. So cache related to all the
tables i.e. table 'tab_conc1', 'tcount' tables will be invalidated.
7. COMMIT the transaction in S1.

The performance in this case is:
No. of tables      | With patch (in ms) | With head (in ms)
-----------------------------------------------------------------------------
tcount = 100      | 101376.4               | 101357.8
tcount = 1000    | 994085.4               | 993471.4

For 100 tables the performance is slow by '0.018%' and for 1000 tables
performance is slow by '0.06%'.
These results are the average of 5 runs.

Other than this I tested the following cases but did not find any
performance impact:
1. with 'tcount = 10'. But I didn't find any performance impact.
2. with 'tcount = 0' and running the loop 1000 times. But I didn't
find any performance impact.

I have also attached the test script and the machine configurations on
which performance testing was done.
Next I am planning to test solely on the logical decoding side and
will share the results.

Thanks and Regards,
Shlok Kyal