Re: Enhancing Memory Context Statistics Reporting

From: Rahila Syed <rahilasyed90(at)gmail(dot)com>
To: PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Cc: Fujii Masao <masao(dot)fujii(at)oss(dot)nttdata(dot)com>, Tomas Vondra <tomas(at)vondra(dot)me>, Andres Freund <andres(at)anarazel(dot)de>, torikoshia <torikoshia(at)oss(dot)nttdata(dot)com>
Subject: Re: Enhancing Memory Context Statistics Reporting
Date: 2025-02-18 13:05:38
Message-ID: CAH2L28sNHmj+aCJ=XkNb0an-XAs3eOUcU4orx2G9yviRL560fg@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi,

>
> Thanks for updating the patch!
>
> The below comments would be a bit too detailed at this stage, but I’d
> like to share the points I noticed.
>
> Thanks for sharing the detailed comments. I have incorporated some of them
into the new version of the patch. I will include the rest when I refine and
comment the code further.

Meanwhile, I have fixed the following outstanding issues:

1. Currently one DSA is created per backend when the first request for
> statistics is made and remains for the lifetime of the server.
> I think I should add logic to periodically destroy DSAs, when memory
> context statistics are not being *actively* queried from the backend,
> as determined by the statistics timestamp.
>

After an offline discussion with Andres and Tomas, I have fixed this to use
only one DSA for all the publishing backends/processes. Each backend
allocates smaller chunks of memory within the DSA while publishing
statistics.
These chunks are tracked independently by each backend, ensuring that two
publishing backends/processes do not block each other despite using the
same
DSA. This approach eliminates the overhead of creating multiple DSAs,
one for each backend.

I am not destroying the DSA area because it stores the previously published
statistics for each process. This allows the system to display older
statistics
when the latest data cannot be retrieved within a reasonable time.
Only the most recently updated statistics are kept, while all earlier ones
are freed using dsa_free by each backend when they are no longer needed.
.

> 2. The two issues reported by Fujii-san here: [1].
> i. I have proposed a fix for the first issue here [2].
> ii. I am able to reproduce the second issue. This happens when we try
> to query statistics of a backend running infinite_recurse.sql. While I am
> working on finding a root-cause, I think it happens due to some memory
> being overwritten due to to stack-depth violation, as the issue is not
> seen
> when I reduce the max_stack_depth to 100kb.
> }
> }
>

The second issue is also resolved by using smaller allocations within a
DSA.
Previously, it occurred because a few statically allocated strings were
placed
within a single large chunk of DSA allocation. I have changed this to use
dynamically allocated chunks with dsa_allocate0 within the same DSA.

Please find attached updated and rebased patches.

Thank you,
Rahila Syed

Attachment Content-Type Size
v13-0001-Preparatory-changes-for-reporting-memory-context-sta.patch application/octet-stream 4.3 KB
v13-0002-Function-to-report-memory-context-statistics.patch application/octet-stream 50.3 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Dagfinn Ilmari Mannsåker 2025-02-18 13:11:11 Re: test_escape: invalid option -- 'c'
Previous Message Ranier Vilela 2025-02-18 12:58:46 Improve cleaning files on Postgres crashes