From: | "Hayato Kuroda (Fujitsu)" <kuroda(dot)hayato(at)fujitsu(dot)com> |
---|---|
To: | 'Masahiko Sawada' <sawada(dot)mshk(at)gmail(dot)com> |
Cc: | "Zhijie Hou (Fujitsu)" <houzj(dot)fnst(at)fujitsu(dot)com>, Xuneng Zhou <xunengzhou(at)gmail(dot)com>, 赵宇鹏(宇彭) <zhaoyupeng(dot)zyp(at)alibaba-inc(dot)com>, pgsql-hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
Subject: | RE: memory leak in logical WAL sender with pgoutput's cachectx |
Date: | 2025-08-21 05:23:07 |
Message-ID: | OSCPR01MB14966E0225BC4AA1BC956E1E3F532A@OSCPR01MB14966.jpnprd01.prod.outlook.com |
Views: | Whole Thread | Raw Message | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
Dear Sawada-san,
> 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?
Oh, right. I tried to handle the case that invalidated entries are re-validated
again, the approach was not correct.
> 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?).
Exactly.
> 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.
Firstly I also considered but did not choose because of the code complexity.
After considering more, it is not so difficult, PSA new file.
It introduces a global dclist invalidated_caches and a its node in RelationSyncEntry.
RelSyncEntry can be pushed to the list when it is invalidated, and the length is
checked at the end of txn. Listed entries can be released if the length exceeds
the threshold.
A flag "already_pushed" is also needed to avoid queuing the same entry twice.
There is a possibility that same entry can be invalidated twice before cleaning
up it. I spent much time to recognize it :-(.
I'm planning to do a benchmark with this patch.
Best regards,
Hayato Kuroda
FUJITSU LIMITED
Attachment | Content-Type | Size |
---|---|---|
v3-0001-pgoutput-allow-cleaning-up-Relsync-cache-entries.patch | application/octet-stream | 14.8 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Hayato Kuroda (Fujitsu) | 2025-08-21 05:27:03 | RE: memory leak in logical WAL sender with pgoutput's cachectx |
Previous Message | Thomas Munro | 2025-08-21 05:15:25 | Re: VM corruption on standby |