Re: Patch: New GUC prepared_statement_limit to limit memory used by prepared statements

Am 17.08.2019 um 19:10 schrieb Ibrar Ahmed:
> On Sat, Aug 17, 2019 at 6:58 PM Daniel Migowski <dmigowski(at)ikoffice(dot)de
> <mailto:dmigowski(at)ikoffice(dot)de>> wrote:
>
>
> attached you find a patch that adds a new GUC:
>
>
> Quick questions before looking at the patch.
>
>
> prepared_statement_limit:
>
>  - Do we have a consensus about the name of GUC? I don't think it is
> the right name for that.
No, it is a proposal. It could also be named plancache_mem or
cachedplansource_maxmem or anything else. It was intended to make
prepared statements not use up all my mem, but development has shown
that it could also be used for other CachedPlans, as long as it is a
saved plan.
>  - Is this a WIP patch or the final patch? Because I can see TODO and
> non-standard
> comments in the patch.

Definitely work in progress! The current implementation seems to work
for me, but might be improved, but I wanted some input from the mailing
list before I optimize things.

The most important question is, if such a feature would find some love
here. Personally it is essential for me because a single prepared
statement uses up to 45MB in my application and there were cases where
ORM-generated prepared statememts would crash my server after some time.

Then I would like to know if the current implementation would at least
not crash (even it might by slow a bit) or if I have to take more care
for locking in some places. I believe a backend is a single thread of
execution but there were notes of invalidation messages that seem to be
run asynchronously to the main thread. Could someone care to explain the
treading model of a single backend to me? Does it react so signals and
what would those signals change? Can I assume that only the
ResetPlanCache, PlanCacheRelCallback and PlanCacheObjectCallback will be
called async?

>          Specifies the maximum amount of memory used in each
> session to
> cache
>          parsed-and-rewritten queries and execution plans. This
> affects
> the maximum memory
>          a backend threads will reserve when many prepared statements
> are used.
>          The default value of 0 disables this setting, but it is
> recommended to set this
>          value to a bit lower than the maximum memory a backend
> worker
> thread should reserve
>          permanently.
>
> If the GUC is configured after each save of a CachedPlanSource, or
> after
> creating a CachedPlan from it, the function
> EnforcePreparedStatementLimit is called now. It checks the mem
> usage of
> the existing saved CachedPlanSources and invalidates the
> query_list and
> the gplan if available until the memory limit is met again.
>
> CachedPlanSource are removed-and-tailadded in the saved_plan_list
> everytime GetCachedPlan is called on them so it can be used as a
> LRU list.
>
Could be a single move-to-tail function I would add.
>
> I also reworked ResetPlanCache, PlanCacheRelCallback and
> PlanCacheObjectCallback a bit so when a CachedPlanSource is
> invalidated
> the query_list is not only marked as invalid but it is also fully
> released to free memory here.
>
Because this seems to work I was able to reuse the new ReleaseQueryList
in my implementation.
>
>
> Regards,
> Daniel Migowski
>
> PS(at)Konstantin: This patch also includes the CachedPlanMemoryUsage
> function you like, maybe you like the review the patch for me?
>
> --
> Ibrar Ahmed
Daniel Migowski