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

Amit Kapila <amit.kapila16@gmail.com>

From: Amit Kapila <amit.kapila16@gmail.com>

To: Nitin Motiani <nitinmotiani@google.com>

Cc: vignesh C <vignesh21@gmail.com>, Tomas Vondra <tomas.vondra@enterprisedb.com>, Andres Freund <andres@anarazel.de>, PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>

Date: 2024-07-16T06:29:04Z

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 →

Fix typo in test file name added in commit 4909b38af0.
- 50b8ad30f754 18.0 landed
- d96206f259d6 17.5 landed
- 9987c94662c2 16.9 landed
- 90bc4523fd47 15.13 landed
- bb1bc9fa962e 14.18 landed
- 4164d6976316 13.21 landed
Fix data loss in logical replication.
- 247ee94150b6 13.21 landed
- 4909b38af034 18.0 landed
- cadaf0ac4637 17.5 landed
- 9a2f8b4f01d5 16.9 landed
- 9f21be08e884 15.13 landed
- 0434033e8bb5 14.18 landed
Avoid invalidating all RelationSyncCache entries on publication rename.
- 3abe9dc18892 18.0 cited
Remove obsolete RECHECK keyword completely
- 7da1bdc2c2f1 18.0 cited
Backport BackgroundPsql perl test module
- 187b8991f70f 16.4 cited

On Tue, Jul 16, 2024 at 9:29 AM Amit Kapila <amit.kapila16@gmail.com> wrote:
>
> One related comment:
> @@ -1219,8 +1219,14 @@ AlterPublicationTables(AlterPublicationStmt
> *stmt, HeapTuple tup,
>   oldrel = palloc(sizeof(PublicationRelInfo));
>   oldrel->whereClause = NULL;
>   oldrel->columns = NIL;
> +
> + /*
> + * Data loss due to concurrency issues are avoided by locking
> + * the relation in ShareRowExclusiveLock as described atop
> + * OpenTableList.
> + */
>   oldrel->relation = table_open(oldrelid,
> -   ShareUpdateExclusiveLock);
> +   ShareRowExclusiveLock);
>
> Isn't it better to lock the required relations in RemovePublicationRelById()?
>

On my CentOS VM, the test file '100_bugs.pl' takes ~11s without a
patch and ~13.3s with a patch. So, 2 to 2.3s additional time for newly
added tests. It isn't worth adding this much extra time for one bug
fix. Can we combine table and schema tests into one single test and
avoid inheritance table tests as the code for those will mostly follow
the same path as a regular table?

-- 
With Regards,
Amit Kapila.