Re: Align memory context level numbering in pg_log_backend_memory_contexts()

From: David Rowley <dgrowleyml(at)gmail(dot)com>
To: torikoshia <torikoshia(at)oss(dot)nttdata(dot)com>
Cc: Daniel Gustafsson <daniel(at)yesql(dot)se>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Align memory context level numbering in pg_log_backend_memory_contexts()
Date: 2025-04-16 22:31:57
Message-ID: CAApHDvrAQPuwOe5KZ9Kb3Lo68eQJJxaJW5_bBCf4-FwudNy2Sg@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Thu, 17 Apr 2025 at 02:20, torikoshia <torikoshia(at)oss(dot)nttdata(dot)com> wrote:
> Regarding the implementation:
> In the initial patch attached, I naïvely incremented the level just
> before emitting the log line.
> However, it might be cleaner to simply initialize the level variable to
> 1 from the start. This could help avoid unnecessary confusion when
> debugging that part of the code.

I didn't look at your patch before, but have now and agree that's not
the best way.

> Similarly, I noticed that in pg_get_process_memory_contexts(), the level
> is initialized to 0 in ProcessGetMemoryContextInterrupt(void):
>
> int level = 0;
> ..
> MemoryContextStatsInternal(c, level, 100, 100, &grand_totals, ..
>
> If we want to be consistent, perhaps it would make sense to start from 1
> there as well.

Yes.

> BTW level variable has existed since before pg_backend_memory_contexts
> was introduced — it was originally used for functions that help inspect
> memory contexts via the debugger. Because of that, I think changing this
> would affect not only these functions codes but some older ones.

I get the impression that it wasn't well thought through prior to
this. If you asked for max_level of 10 it would stop at 9. Changing
these to 1-based levels means we'll now stop at level 10 without
printing any more levels than we did before.

The attached patch is how I think we should do it.

David

Attachment Content-Type Size
make_memory_context_levels_1_based.patch application/octet-stream 1.7 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Tom Lane 2025-04-16 23:04:50 Re: jsonapi: scary new warnings with LTO enabled
Previous Message Michael Paquier 2025-04-16 22:29:53 Re: pg_dump: Fix dangling pointer in EndCompressorZstd()