Re: Adding column "mem_usage" to view pg_prepared_statements

Am 05.08.2019 um 18:30 schrieb Konstantin Knizhnik:
> On 31.07.2019 1:38, Daniel Migowski wrote:
>> Am 31.07.2019 um 00:29 schrieb Tom Lane:
>>> Daniel Migowski <dmigowski@ikoffice.de> writes:
>>>> Ok, just have read about the commitfest thing. Is the patch OK for 
>>>> that? Or is there generally no love for a mem_usage column here? If 
>>>> it was, I really would add some memory monitoring in our app 
>>>> regarding this.
>>> You should certainly put it into the next commitfest.  We might or
>>> might not accept it, but if it's not listed in the CF we might
>>> not remember to even review it.  (The CF app is really a to-do
>>> list for patches ...)
>> OK, added it there. Thanks for your patience :).
> The patch is not applied to the most recent source because extra 
> parameter was added to CreateTemplateTupleDesc method.
> Please rebase it - the fix is trivial.
OK, will do.
> I think that including in pg_prepared_statements information about 
> memory used this statement is very useful.
> CachedPlanMemoryUsage function may be useful not only for this view, 
> but for example it is also need in my autoprepare patch.
I would love to use your work if it's done, and would also love to work 
together here. I am quite novice in C thought, I might take my time to 
get things right.
> I wonder if you consider go further and not only report but control 
> memory used by prepared statements?
> For example implement some LRU replacement discipline on top of 
> prepared statements cache which can
> evict rarely used prepared statements to avoid memory overflow.

THIS! Having some kind of safety net here would finally make sure that 
my precious processes will not grow endlessly until all mem is eaten up, 
even with prep statement count limits.

While working on stuff I noticed there are three things stored in a 
CachedPlanSource. The raw query tree (a relatively small thing), the 
query list (analyzed-and-rewritten query tree) which takes up the most 
memory (at least here, maybe different with your usecases), and (often 
after the 6th call) the CachedPlan, which is more optimized that the 
query list and often needs less memory (half of the query list here).

The query list seems to take the most time to create here, because I hit 
the GEQO engine here, but it could be recreated easily (up to 500ms for 
some queries). Creating the CachedPlan afterwards takes 60ms in some 
usecase. IF we could just invalidate them from time to time, that would 
be grate.

Also, invalidating just the queries or the CachedPlan would not 
invalidate the whole prepared statement, which would break clients 
expectations, but just make them a slower, adding much to the stability 
of the system. I would pay that price, because I just don't use manually 
named prepared statements anyway and just autogenerate them as 
performance sugar without thinking about what really needs to be 
prepared anyway. There is an option in the JDBC driver to use prepared 
statements automatically after you have used them a few time.

> We have such patch for PgPro-EE but it limits only number of prepared 
> statement, not taken in account amount of memory used by them.
> I think that memory based limit will be more accurate (although it 
> adds more overhead).

Limiting them by number is already done automatically here and would 
really not be of much value, but having a mem limit would be great. We 
could have a combined memory limit for your autoprepared statements as 
well as the manually prepared ones, so clients can know for sure the 
server processes won't eat up more that e.g. 800MB for prepared 
statements. And also I would like to have this value spread across all 
client processes, e.g. specifying max_prepared_statement_total_mem=5G 
for the server, and maybe max_prepared_statement_mem=1G for client 
processes. Of course we would have to implement cross client process 
invalidation here, and I don't know if communicating client processes 
are even intended.

Anyway, a memory limit won't really add that much more overhead. At 
least not more than having no prepared statements at all because of the 
fear of server OOMs, or have just a small count of those statements. I 
was even think about a prepared statement reaper that checks the 
pg_prepared_statements every few minutes to clean things up manually, 
but having this in the server would be of great value to me.

> If you want, I can be reviewer of your patch.

I'd love to have you as my reviewer.

Regards,
Daniel Migowski