Re: memory leak in logical WAL sender with pgoutput's cachectx

On Sun, Aug 17, 2025 at 11:30 PM Hayato Kuroda (Fujitsu)
<kuroda.hayato@fujitsu.com> wrote:
>
> Dear Sawada-san,
>
> > I've not verified, but even if that's true, IIUC only one relation's
> > cache entry can set in_use to true at a time.
>
> I also think so.
>
> > If my understanding is
> > correct, when the walsender accepts invalidation messages in
> > logicalrep_write_tuple() as you mentioned, it doesn't release the
> > cache of the relation whose change is being sent but does for other
> > relations. It seems to me too aggressive to release caches compared to
> > the current behavior.
>
> Valid point. In some cases, publications can be altered then same relsync entries
> would be added again.
>
> So... first approach may be better from the perspective. Attached patch counts
> the number of invalidated entries. There is a chance to cleanup them only at
> COMMIT/ABORT/PREPARE time - hence in_use flag is not needed. Thought?
>

In the proposed patch, we have the following change:

@@ -2060,6 +2081,11 @@ get_rel_sync_entry(PGOutputData *data, Relation relation)
        entry->columns = NULL;
        entry->attrmap = NULL;
    }
+   else
+   {
+       /* The entry would be re-used, decrement the counter */
+       RelationSyncCacheInvalidateCounter--;
+   }

It decrements the counter whenever we successfully find the entry from
the cache but I'm not sure this is the right approach. What if no
cache invalidation happens at all but we retrieve entries from the
cache many times?

I have concerns about the performance implications of iterating
through all entries in the caches within
maybe_cleanup_rel_sync_cache(). If the cache contains numerous
entries, this iteration could potentially cause the walsender to
stall. If we use a larger number NINVALIDATION_THRESHOLD, we can
reduce the number of times we need sequential scans on the hash table
but it would in turn need to free more entries (probably we can have a
cap of the number of entries we can free in one cycle?).

An alternative approach would be to implement a dedicated list (such
as dclist) specifically for tracking invalidated entries. Entries
would be removed from this list when they are reused. We could then
implement a threshold-based cleanup mechanism where invalidated
entries are freed once the list exceeds a predetermined size. While
this approach would minimize the overhead of freeing invalidated
entries, it would incur some additional cost for maintaining the list.

Regards,

--
Masahiko Sawada
Amazon Web Services: https://aws.amazon.com