Re: Enhancing Memory Context Statistics Reporting - Mailing list pgsql-hackers

On Fri, Nov 22, 2024 at 6:33 PM Rahila Syed <rahilasyed90@gmail.com> wrote:
>
> Hi,
>
>> How does the process know that the client backend has finished reading
>> stats and it can be refreshed? What happens, if the next request for
>> memory context stats comes before first requester has consumed the
>> statistics it requested?
>>
> A process that's copying its statistics does not need to know that.
> Whenever it receives a signal to copy statistics, it goes ahead and
> copies the latest statistics to the DSA after acquiring an exclusive
> lwlock.
>
> A requestor takes a lock before it starts consuming the statistics.
> If the next request comes while the first requestor is consuming the
> statistics, the publishing process will wait on lwlock to be released
> by the consuming process before it can write the statistics.
> If the next request arrives before the first requester begins consuming
> the statistics, the publishing process will acquire the lock and overwrite
> the earlier statistics with the most recent ones.
> As a result, both the first and second requesters will consume the
> updated statistics.

IIUC, the publisher and the consumer processes, both, use the same
LWLock. Publisher acquires an exclusive lock. Does consumer acquire
SHARED lock?

The publisher process might be in a transaction, processing a query or
doing something else. If it has to wait for an LWLock may affect its
performance. This will become even more visible if the client backend
is trying to diagnose a slow running query. Have we tried to measure
how long the publisher might have to wait for an LWLock while the
consumer is consuming statistics OR what is the impact of this wait?

>> >
>> > When statistics of a local backend is requested, this function returns the following
>> > WARNING and exits, since this can be handled by an existing function which
>> > doesn't require a DSA.
>> >
>> > WARNING:  cannot return statistics for local backend
>> > HINT:  Use pg_get_backend_memory_contexts instead
>>
>> How about using pg_get_backend_memory_contexts() for both - local as
>> well as other backend? Let PID argument default to NULL which would
>> indicate local backend, otherwise some other backend?
>>
> I don't see much value in combining the two, specially since with
> pg_get_process_memory_contexts() we can query both the postgres
> backend and a background process, the name pg_get_backend_memory_context()
> would be inaccurate and I am not sure whether a change to rename the
> existing function would be welcome.

Having two separate functions for the same functionality isn't a
friendly user interface.

Playing a bit with pg_terminate_backend() which is another function
dealing with backends to understand a. what does it do to its own
backend and b. which processes are considered backends.

1. pg_terminate_backend() allows to terminate the backend from which
it is fired.
#select pid, application_name, backend_type, pg_terminate_backend(pid)
from pg_stat_activity;
FATAL:  terminating connection due to administrator command
server closed the connection unexpectedly
        This probably means the server terminated abnormally
        before or while processing the request.
The connection to the server was lost. Attempting reset: Succeeded.

2. It considers autovacuum launcher and logical replication launcher
as postgres backends but not checkpointer, background writer and
walwriter.
#select pid, application_name, backend_type, pg_terminate_backend(pid)
from pg_stat_activity where pid <> pg_backend_pid();
WARNING:  PID 644887 is not a PostgreSQL backend process
WARNING:  PID 644888 is not a PostgreSQL backend process
WARNING:  PID 644890 is not a PostgreSQL backend process
  pid   | application_name |         backend_type         | pg_terminate_backend
--------+------------------+------------------------------+----------------------
 645636 |                  | autovacuum launcher          | t
 645677 |                  | logical replication launcher | t
 644887 |                  | checkpointer                 | f
 644888 |                  | background writer            | f
 644890 |                  | walwriter                    | f
(5 rows)

In that sense you are correct that pg_get_backend_memory_context()
should not provide context information of WAL writer process for
example. But pg_get_process_memory_contexts() would be expected to
provide its own memory context information instead of redirecting to
another function through a WARNING. It could do that redirection
itself. That will also prevent the functions' output format going out
of sync.

--
Best Wishes,
Ashutosh Bapat