Re: long-standing data loss bug in initial sync of logical replication - Mailing list pgsql-hackers

On Mon, Feb 24, 2025 at 4:49 PM Shlok Kyal <shlok.kyal.oss@gmail.com> wrote:
>
> Patches need a rebase. Attached the rebased patch.
>

I would like to discuss 0002 patch:
 publication_invalidation_cb(Datum arg, int cacheid, uint32 hashvalue)
 {
  publications_valid = false;
-
- /*
- * Also invalidate per-relation cache so that next time the filtering info
- * is checked it will be updated with the new publication settings.
- */
- rel_sync_cache_publication_cb(arg, cacheid, hashvalue);
 }

 /*
@@ -1970,18 +1964,6 @@ init_rel_sync_cache(MemoryContext cachectx)
    rel_sync_cache_publication_cb,
    (Datum) 0);

- /*
- * Flush all cache entries after any publication changes.  (We need no
- * callback entry for pg_publication, because publication_invalidation_cb
- * will take care of it.)
- */
- CacheRegisterSyscacheCallback(PUBLICATIONRELMAP,
-   rel_sync_cache_publication_cb,
-   (Datum) 0);
- CacheRegisterSyscacheCallback(PUBLICATIONNAMESPACEMAP,
-   rel_sync_cache_publication_cb,
-   (Datum) 0);

In 0002 patch, we are improving the performance by avoiding
invalidation processing in a number of cases. Basically, the claim is
that we are unnecessarily invalidating all the RelSyncCache entries
when a particular relation's entry could be invalidated. I have not
verified it, but IIUC, this should be an independent improvement atop
HEAD; if so, then we should start a separate thread to discuss it.

Thoughts?

--
With Regards,
Amit Kapila.