On 2014-08-14 22:16:31 -0700, Michael Paquier wrote:
> And here are some comments about patch 2:
> - Patch applies with some hunks.
> - Some typos are present
> s#memory segments..#memory segments. (double dots)
> s#NULL#<literal>NULL</> (in the docs as this refers to a value)

Will fix.

> - Your thoughts about providing separate patches for each view? What
> this patch does is straight-forward, but pg_shmem_allocations does not
> actually depend on the first patch adding size and name to the dsm
> fields. So IMO it makes sense to separate each feature properly.

I don't know, seems a bit like busywork to me. Postgres doesn't really
very granular commits...

> - off should be renamed to offset for pg_get_shmem_allocations.

ok.

> - Is it really worth showing unused shared memory? I'd rather rip out
> the last portion of pg_get_shmem_allocations.

It's actually really helpful. There's a couple situations where you
possibly can run out of that spare memory and into troubles. Which
currently aren't diagnosable. Similarly we currently can't diagnose
whether we're superflously allocate too much 'reserve' shared memory.

> - For refcnt in pg_get_dynamic_shmem_allocations, could you add a
> comment mentioning that refcnt = 1 means that the item is moribund and
> 0 is unused, and that reference count for active dsm segments only
> begins from 2? I would imagine that this is enough, instead of using
> some define's defining the ID from which a dsm item is considered as
> active.

Ok.

Greetings,

Andres Freund

-- 
 Andres Freund                     http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to