Re: Parent/child context relation in pg_get_backend_memory_contexts()

On Tue, 16 Jul 2024 at 06:19, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
> I'm not against what you're trying to do here, but I feel like you
> might be over-engineering it. I don't think there was anything really
> wrong with what Melih was doing, and I don't think there's anything
> really wrong with converting the path to an array of strings, either.
> Sure, it might not be perfect, but future patches could always remove
> the name duplication. This is a debugging facility that will be used
> by a tiny minority of users, and if some non-uniqueness gets
> reintroduced in the future, it's not a critical defect and can just be
> fixed when it's noticed.

I'm just not on board with the
query-returns-correct-results-most-of-the-time attitude and I'm
surprised you are. You can get that today if you like, just write a
WITH RECURSIVE query joining "name" to "parent". If the consensus is
that's fine because it works most of the time, then I don't see any
reason to invent a new way to get equally correct-most-of-the-time
results.

> That said, if you want to go with the integer
> IDs and want to spend more time massaging it, I also think that's
> fine. I simply don't believe it's the only way forward here. YMMV, but
> my opinion is that none of these approaches have such critical flaws
> that we need to get stressed about it.

If there are other ways forward that match the goal of having a
reliable way to determine the parent of a MemoryContext, then I'm
interested in hearing more. I know you've mentioned about having
unique names, but I don't know how to do that. Do you have any ideas
on how we could enforce the uniqueness? I don't really like your idea
of renaming contexts when we find duplicate names as bug fixes. The
nature of our code wouldn't make it easy to determine as some reusable
code might create a context as a child of CurrentMemoryContext and
multiple callers might call that code within a different
CurrentMemoryContext.

One problem is that, if you look at MemoryContextCreate(), we require
that the name is statically allocated. We don't have the flexibility
to assign unique names when we find a conflict. If we were to come up
with a solution that assigned a unique name, then I'd call that
"over-engineered" for the use case we need it for. I think if we did
something like that, it would undo some of the work Tom did in
442accc3f. Also, I think it was you that came up with the idea of
MemoryContext reuse (9fa6f00b1)? Going by that commit message, it
seems to be done for performance reasons. If MemoryContext.name was
dynamic, there'd be more allocation work to do when reusing a context.
That might undo some of the performance gains seen in 9fa6f00b1. I
don't really want to go through the process of verifying there's no
performance regress for a patch that aims to make
pg_backend_memory_contexts more useful.

David