Message-ID: <4c2b5401-41ad-43b9-9215-c7b2d901f8ed@vondra.me>
Date: Fri, 21 Feb 2025 16:01:00 +0100
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: Enhancing Memory Context Statistics Reporting
To: Rahila Syed <rahilasyed90@gmail.com>,
 PostgreSQL-development <pgsql-hackers@postgresql.org>
Cc: Daniel Gustafsson <daniel@yesql.se>
References: <ZzV-8qOOBtPdP4Tq@paquier.xyz>
 <7d4db5cf-8d72-4ee1-8f2e-f88ef9615f27@vondra.me>
 <CAH2L28uvYHX1CfmbcB9JPzR6+MzJ+kb9ob+NvFy3wUHM8HUkGA@mail.gmail.com>
 <637b3ae3-b9eb-4d83-a942-5f7483911ea4@vondra.me>
 <CAH2L28spk4c54k5pTSWRqu5LJmetQwTXtr=-F7Bnioxd425hqQ@mail.gmail.com>
 <29ced66e28ec696d1ac29747bd901df2@oss.nttdata.com>
 <CAH2L28vEiz+Qxw=Y5Dze8_TxDW1kAyh_3R-BKUGvOH6r02W6cg@mail.gmail.com>
 <5a899047-65fc-47c5-9c35-1035f44956c6@vondra.me>
 <CAH2L28vR622wV44XenbhWc7ETpNtjS_oeTha7OxMx35LjWPPqQ@mail.gmail.com>
 <70146bd0-da0f-48ee-bece-1e35536189e0@oss.nttdata.com>
 <CAH2L28u7=fcgnY8bpM87moiJxt++wqWZXh2HxFabYjiHSg76Cg@mail.gmail.com>
 <2eb8fd76-89f0-42e8-bc79-152d50c63016@vondra.me>
 <CAH2L28vYvC5DP+YccBE7VnC-khn3k_vd=MGxn8BFEpLc40ncHw@mail.gmail.com>
 <81607c8c04a21b5cac87b21e46566074@oss.nttdata.com>
 <CAH2L28sNHmj+aCJ=XkNb0an-XAs3eOUcU4orx2G9yviRL560fg@mail.gmail.com>
 <CAH2L28sMnNy9DvzAAoiE8qQs0MRX9ALhaYAf2f-aLivL47Ryhw@mail.gmail.com>
Content-Language: en-US
From: Tomas Vondra <tomas@vondra.me>
In-Reply-To: <CAH2L28sMnNy9DvzAAoiE8qQs0MRX9ALhaYAf2f-aLivL47Ryhw@mail.gmail.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Archived-At: <https://www.postgresql.org/message-id/4c2b5401-41ad-43b9-9215-c7b2d901f8ed%40vondra.me>
Precedence: bulk

On 2/20/25 14:26, Rahila Syed wrote:
> Hi,
> 
> Please find attached the updated patches after some cleanup and test
> fixes.  
> 
> Thank you,
> Rahila Syed
> 
> On Tue, Feb 18, 2025 at 6:35 PM Rahila Syed <rahilasyed90@gmail.com
> <mailto:rahilasyed90@gmail.com>> wrote:
> 
>     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.
>     .  

I think something is not quite right, because if I try running a simple
pgbench script that does pg_get_process_memory_contexts() on PIDs of
random postgres process (just like in the past), I immediately get this:

pgbench: error: client 28 script 0 aborted in command 0 query 0: ERROR:
can't attach the same segment more than once
pgbench: error: client 10 script 0 aborted in command 0 query 0: ERROR:
can't attach the same segment more than once
pgbench: error: client 5 script 0 aborted in command 0 query 0: ERROR:
can't attach the same segment more than once
pgbench: error: client 8 script 0 aborted in command 0 query 0: ERROR:
can't attach the same segment more than once
...

Perhaps the backends need to synchronize creation of the DSA?

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

Sounds good. Do you have any measurements how much this reduced the size
of the entries written to the DSA? How many entries will fit into 1MB of
shared memory?


regards

-- 
Tomas Vondra