Re: Logical replication ApplyContext bloat

On 19/04/17 11:55, Stas Kelvich wrote:
>
>> On 19 Apr 2017, at 12:37, Petr Jelinek <petr(dot)jelinek(at)2ndquadrant(dot)com> wrote:
>>
>> On 18/04/17 13:45, Stas Kelvich wrote:
>>> Hi,
>>>
>>> currently logical replication worker uses ApplyContext to decode received data
>>> and that context is never freed during transaction processing. Hence if publication
>>> side is performing something like 10M row inserts in single transaction, then
>>> subscription worker will occupy more than 10G of ram (and can be killed by OOM).
>>>
>>> Attached patch resets ApplyContext after each insert/update/delete/commit.
>>> I’ve tried to reset context only on each 100/1000/10000 value of CommandCounter,
>>> but didn’t spotted any measurable difference in speed.
>>>
>>> Problem spotted by Mikhail Shurutov.
>>>
>>
>> Hmm we also do replication protocol handling inside the ApplyContext
>> (LogicalRepApplyLoop), are you sure this change is safe in that regard?
>
> Memory is bloated by logicalrep_read_* when palloc happens inside.
> I’ve inserted context reset in ensure_transaction() which is only called in beginning
> of hande_insert/update/delete/commit when data from previous call of that
> function isn’t needed. So probably it is safe to clear memory there. At least
> i don’t see any state that can be accessed later in this functions. Do you
> have any specific concerns?
>

I wanted to make sure you checked things that are happening outside of
the apply_handle_* stuff. I checked myself now, the allocations that
happen outside don't use postgres memory contexts (libpqrcv_receive) so
that should not be problem. Other allocations that I see in neighboring
code switch to permanent contexts anyway. So looks safe on first look
indeed. Eventually we'll want to cache some of the executor stuff so
this will be problem but not in v10.

I'd still like you to add comment to the ApplyContext saying that it's
cleaned after handling each message so nothing that needs to survive for
example whole transaction can be allocated in it as that's not very well
visible IMHO (and I know I will forget about it when writing next patch
in that area).

--
Petr Jelinek http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services