Re: Logical replication ApplyContext bloat

> 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.

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.

>
> --
> Simon Riggs http://www.2ndQuadrant.com/
> PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Stas Kelvich
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company