Horton

Re: Decoding speculative insert with toast leaks memory

Tomas Vondra <tomas.vondra@enterprisedb.com>

View thread View on postgr.es
From: Tomas Vondra <tomas.vondra@enterprisedb.com>
To: Dilip Kumar <dilipbalaut@gmail.com>, Amit Kapila <amit.kapila16@gmail.com>
Cc: Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>, PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>, Andres Freund <andres@anarazel.de>, Petr Jelinek <petr.jelinek@enterprisedb.com>, Peter Eisentraut <peter.eisentraut@2ndquadrant.com>
Date: 2021-05-28T11:46:47Z
Lists: pgsql-hackers 

On 5/27/21 6:36 AM, Dilip Kumar wrote:
> On Thu, May 27, 2021 at 9:47 AM Amit Kapila <amit.kapila16@gmail.com> wrote:
>>
>> On Thu, May 27, 2021 at 9:40 AM Dilip Kumar <dilipbalaut@gmail.com> wrote:
>>
>> True, but if you do this clean-up in ReorderBufferCleanupTXN then you
>> don't need to take care at separate places. Also, toast_hash is stored
>> in txn so it appears natural to clean it up in while releasing TXN.
> 
> Make sense, basically, IMHO we will have to do in TruncateTXN and
> ReturnTXN as attached?
> 

Yeah, I've been working on a fix over the last couple days (because of a
customer issue), and I ended up with the reset in ReorderBufferReturnTXN
too - it does solve the issue in the case I've been investigating.

I'm not sure the reset in ReorderBufferTruncateTXN is correct, though.
Isn't it possible that we'll need the TOAST data after streaming part of
the transaction? After all, we're not resetting invalidations, tuplecids
and snapshot either ... And we'll eventually clean it after the streamed
transaction gets commited (ReorderBufferStreamCommit ends up calling
ReorderBufferReturnTXN too).

I wonder if there's a way to free the TOASTed data earlier, instead of
waiting until the end of the transaction (as this patch does). But I
suspect it'd be way more complex, harder to backpatch, and destroying
the hash table is a good idea anyway.

Also, I think the "if (txn->toast_hash != NULL)" checks are not needed,
because it's the first thing ReorderBufferToastReset does.



regards

-- 
Tomas Vondra
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

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.