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 |
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() |