Re: Logical replication ApplyContext bloat

On 19/04/17 12:46, Stas Kelvich wrote:
>
>> On 19 Apr 2017, at 13:34, Simon Riggs <simon(at)2ndquadrant(dot)com> wrote:
>>
>> On 19 April 2017 at 11:24, Petr Jelinek <petr(dot)jelinek(at)2ndquadrant(dot)com> wrote:
>>
>>> 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).
>>
>> It would be better to invent the contexts we want now, so we get the
>> architecture right for future work. Otherwise we have problems with
>> "can't backpatch this fix because that infrastructure doesn't exist in
>> PG10" in a couple of years.
>>
>> So I suggest we have
>>
>> ApplyMessageContext - cleaned after every message
>> ApplyTransactionContext - cleaned at EOXact
>> ApplyContext - persistent
>
> 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.

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