| 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: | Whole Thread | Raw Message | 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() |