On 2020-07-08 22:12, Fujii Masao wrote:
Thanks for updating the patch! It basically looks good to me.
+ <indexterm zone="view-pg-backend-memory-contexts">
+ <primary>backend memory contexts</primary>
+ </indexterm>
Do we need this indexterm?
Thanks! it's not necessary. I remove this indexterm.
+{ oid => '2282', descr => 'statistics: information about all memory
contexts of local backend',
Isn't it better to remove "statistics: " from the above description?
Yeah, it's my oversight.
+ <row>
+ <entry role="catalog_table_entry"><para
role="column_definition">
+ <structfield>parent</structfield> <type>text</type>
There can be multiple memory contexts with the same name. So I'm
afraid
that it's difficult to identify the actual parent memory context from
this
"parent" column. This is ok when logging memory contexts by calling
MemoryContextStats() via gdb. Because child memory contexts are
printed
just under their parent, with indents. But this doesn't work in the
view.
To identify the actual parent memory or calculate the memory contexts
tree
from the view, we might need to assign unique ID to each memory
context
and display it. But IMO this is overkill. So I'm fine with current
"parent"
column. Thought? Do you have any better idea?
Indeed.
I also feel it's not usual to assign a unique ID, which
can vary every time the view displayed.
Agreed. Displaying such ID would be more confusing to users.
Ok, let's leave the code as it is.
Another comment about parent column is: dynahash can be parent?
If yes, its indent instead of name should be displayed in parent
column?
I'm not sure yet, but considering the changes in the future, it seems
better to do so.
But if we add information for identifying parent-child relation like the
memory address suggested from Andres, it seems not necessary.
So I'd like to go back to this point.
Regards,
--
Atsushi Torikoshi
NTT DATA CORPORATION
