Horton

Re: Decoding speculative insert with toast leaks memory

Amit Kapila <amit.kapila16@gmail.com>

View thread View on postgr.es
From: Amit Kapila <amit.kapila16@gmail.com>
To: Tom Lane <tgl@sss.pgh.pa.us>, Amit Langote <amitlangote09@gmail.com>, osumi.takamichi@fujitsu.com
Cc: Dilip Kumar <dilipbalaut@gmail.com>, Alvaro Herrera <alvherre@alvh.no-ip.org>, Andres Freund <andres@anarazel.de>, Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>, Peter Eisentraut <peter.eisentraut@2ndquadrant.com>, Petr Jelinek <petr.jelinek@enterprisedb.com>, PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>, Tomas Vondra <tomas.vondra@enterprisedb.com>
Date: 2021-06-17T03:56:00Z
Lists: pgsql-hackers 
On Wed, Jun 16, 2021 at 8:18 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>
> Amit Kapila <amit.kapila16@gmail.com> writes:
> > Pushed!
>
> skink reports that this has valgrind issues:
>
> https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=skink&dt=2021-06-15%2020%3A49%3A26
>

The problem happens at line:
rel_sync_cache_relation_cb()
{
..
if (entry->map)
..

I think the reason is that before we initialize 'entry->map' in
get_rel_sync_entry(), the invalidation is processed as part of which
when we try to clean up the entry, it tries to access uninitialized
value. Note, this won't happen in HEAD as we initialize 'entry->map'
before we get to process any invalidation. We have fixed a similar
issue in HEAD sometime back as part of the commit 69bd60672a, so we
need to make a similar change in PG-13 as well.

This problem is introduced by commit d250568121 (Fix memory leak due
to RelationSyncEntry.map.) not by the patch in this thread, so keeping
Amit L and Osumi-San in the loop.

-- 
With Regards,
Amit Kapila.

Commits

  1. Fix ABI break introduced by commit 4daa140a2f.

  2. Fix valgrind issue in pgoutput.c.

  3. Fix decoding of speculative aborts.

  4. pgoutput: Fix memory leak due to RelationSyncEntry.map.

  5. Fix initialization of RelationSyncEntry for streaming transactions.

  6. Implement streaming mode in ReorderBuffer.