Re: Logical replication ApplyContext bloat

> On 19 Apr 2017, at 14:30, Petr Jelinek <petr(dot)jelinek(at)2ndquadrant(dot)com> wrote:
>
> On 19/04/17 12:46, Stas Kelvich wrote:
>>
>> Right now ApplyContext cleaned after each transaction and by this patch I basically
>> suggest to clean it after each command counter increment.
>>
>> So in your definitions that is ApplyMessageContext, most short-lived one. We can
>> rename now ApplyContext -> ApplyMessageContext to make that clear and avoid
>> possible name conflict when somebody will decide to keep something during whole
>> transaction or worker lifespan.
>
> Yeah we can rename, and then rename the ApplyCacheContext to
> ApplyContext. That would leave the ApplyTransactionContext from Simon's
> proposal which is not really need it anywhere right now and I am not
> sure it will be needed given there is still TopTransactionContext
> present. If we really want ApplyTransactionContext we can probably just
> assign it to TopTransactionContext in ensure_transaction.
>
>>
>> P.S. It also seems to me now that resetting context in ensure_transaction() isn’t that
>> good idea, probably better just explicitly call reset at the end of each function involved.
>>
>
> Well it would definitely improve clarity.
>

Okay, done.

With patch MemoryContextStats() shows following hierarchy during slot operations in
apply worker:

TopMemoryContext: 83824 total in 5 blocks; 9224 free (8 chunks); 74600 used
ApplyContext: 8192 total in 1 blocks; 6520 free (4 chunks); 1672 used
ApplyMessageContext: 8192 total in 1 blocks; 6632 free (11 chunks); 1560 used
ExecutorState: 8192 total in 1 blocks; 7624 free (0 chunks); 568 used
ExprContext: 0 total in 0 blocks; 0 free (0 chunks); 0 used