Re: [HACKERS] pg_shmem_allocations view

[Michael Paquier]

On Fri, May 6, 2016 at 8:01 AM, Andres Freund  wrote:
> Here's a rebased version.  I remember why I didn't call the column
> "offset" (as Michael complained about upthread), it's a keyword...

Oh, an old patch resurrected from the dead... Reading again the patch

+* Increase number of initilized slots if we didn't reuse a previously
+* used one.
s/initilized/initialized

+   Number of backends attached to this segment (1 signals a moribund
+   entry, 2 signals one user, ...).
moribund? Or target for removal.

REVOKE ALL .. FROM PUBLIC;
REVOKE EXECUTE .. ON FUNCTION FROM PUBLIC;
Revoking he execution of those views and their underlying functions
would be a good thing I think, this can give hints on the system
activity, particularly for DSM segments.
-- 
Michael


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

Re: [HACKERS] pg_shmem_allocations view

[Andres Freund]

On 2014-09-19 23:07:07 -0500, Michael Paquier wrote:
> On Mon, Aug 18, 2014 at 1:12 PM, Robert Haas  wrote:
> > On Mon, Aug 18, 2014 at 1:27 PM, Tom Lane  wrote:
> >> Robert Haas  writes:
> >>> I thought you were printing actual pointer addresses.  If you're just
> >>> printing offsets relative to wherever the segment happens to be
> >>> mapped, I don't care about that.
> >>
> >> Well, that just means that it's not an *obvious* security risk.
> >>
> >> I still like the idea of providing something comparable to
> >> MemoryContextStats, rather than creating a SQL interface.  The problem
> >> with a SQL interface is you can't interrogate it unless (1) you are not
> >> already inside a query and (2) the client is interactive and under your
> >> control.  Something you can call easily from gdb is likely to be much
> >> more useful in practice.
> >
> > Since the shared memory segment isn't changing at runtime, I don't see
> > this as being a big problem.  It could possibly be an issue for
> > dynamic shared memory segments, though.
> Patch has been reviewed some time ago, extra ideas as well as
> potential security risks discussed as well but no new version has been
> sent, hence marking it as returned with feedback.

Here's a rebased version.  I remember why I didn't call the column
"offset" (as Michael complained about upthread), it's a keyword...

Regards,

Andres
>From 719f7e2493832564c58c3ab319344f31abef1653 Mon Sep 17 00:00:00 2001
From: Andres Freund 
Date: Tue, 6 May 2014 19:42:36 +0200
Subject: [PATCH 1/2] Associate names to created dynamic shared memory
 segments.

This unfortunately implies an API breakage.
---
 src/backend/access/transam/parallel.c |  3 +-
 src/backend/storage/ipc/dsm.c | 61 ++-
 src/include/storage/dsm.h |  2 +-
 src/test/modules/test_shm_mq/setup.c  |  2 +-
 4 files changed, 42 insertions(+), 26 deletions(-)

diff --git a/src/backend/access/transam/parallel.c b/src/backend/access/transam/parallel.c
index 0bba9a7..c1473c1 100644
--- a/src/backend/access/transam/parallel.c
+++ b/src/backend/access/transam/parallel.c
@@ -268,7 +268,8 @@ InitializeParallelDSM(ParallelContext *pcxt)
 	 */
 	segsize = shm_toc_estimate(>estimator);
 	if (pcxt->nworkers != 0)
-		pcxt->seg = dsm_create(segsize, DSM_CREATE_NULL_IF_MAXSEGMENTS);
+		pcxt->seg = dsm_create("parallel worker", segsize,
+			   DSM_CREATE_NULL_IF_MAXSEGMENTS);
 	if (pcxt->seg != NULL)
 		pcxt->toc = shm_toc_create(PARALLEL_MAGIC,
    dsm_segment_address(pcxt->seg),
diff --git a/src/backend/storage/ipc/dsm.c b/src/backend/storage/ipc/dsm.c
index 573c54d..7166328 100644
--- a/src/backend/storage/ipc/dsm.c
+++ b/src/backend/storage/ipc/dsm.c
@@ -80,8 +80,10 @@ struct dsm_segment
 /* Shared-memory state for a dynamic shared memory segment. */
 typedef struct dsm_control_item
 {
-	dsm_handle	handle;
+	dsm_handle	handle;			/* segment identifier */
 	uint32		refcnt;			/* 2+ = active, 1 = moribund, 0 = gone */
+	Size		size;			/* current size */
+	char		name[SHMEM_INDEX_KEYSIZE]; /* informational name */
 } dsm_control_item;
 
 /* Layout of the dynamic shared memory control segment. */
@@ -454,15 +456,18 @@ dsm_set_control_handle(dsm_handle h)
  * Create a new dynamic shared memory segment.
  */
 dsm_segment *
-dsm_create(Size size, int flags)
+dsm_create(const char *name, Size size, int flags)
 {
 	dsm_segment *seg;
-	uint32		i;
-	uint32		nitems;
+	dsm_control_item *item;
+	uint32		slot;
 
 	/* Unsafe in postmaster (and pointless in a stand-alone backend). */
 	Assert(IsUnderPostmaster);
 
+	Assert(name != NULL && strlen(name) > 0 &&
+		   strlen(name) < SHMEM_INDEX_KEYSIZE - 1);
+
 	if (!dsm_init_done)
 		dsm_backend_startup();
 
@@ -482,23 +487,18 @@ dsm_create(Size size, int flags)
 	/* Lock the control segment so we can register the new segment. */
 	LWLockAcquire(DynamicSharedMemoryControlLock, LW_EXCLUSIVE);
 
-	/* Search the control segment for an unused slot. */
-	nitems = dsm_control->nitems;
-	for (i = 0; i < nitems; ++i)
+	/*
+	 * Search the control segment for an unused slot that's previously been
+	 * used. If we don't find one initialize a new one if there's still space.
+	 */
+	for (slot = 0; slot < dsm_control->nitems; ++slot)
 	{
-		if (dsm_control->item[i].refcnt == 0)
-		{
-			dsm_control->item[i].handle = seg->handle;
-			/* refcnt of 1 triggers destruction, so start at 2 */
-			dsm_control->item[i].refcnt = 2;
-			seg->control_slot = i;
-			LWLockRelease(DynamicSharedMemoryControlLock);
-			return seg;
-		}
+		if (dsm_control->item[slot].refcnt == 0)
+			break;
 	}
 
-	/* Verify that we can support an additional mapping. */
-	if (nitems >= dsm_control->maxitems)
+	/* Verify that we can support the mapping. */
+	if (slot >= dsm_control->maxitems)
 	{
 		if ((flags & DSM_CREATE_NULL_IF_MAXSEGMENTS) != 0)
 		{
@@ -516,12 +516,23 @@ dsm_create(Size size, int flags)
  

Re: [HACKERS] pg_shmem_allocations view

[Michael Paquier]

On Mon, Aug 18, 2014 at 1:12 PM, Robert Haas robertmh...@gmail.com wrote:
 On Mon, Aug 18, 2014 at 1:27 PM, Tom Lane t...@sss.pgh.pa.us wrote:
 Robert Haas robertmh...@gmail.com writes:
 I thought you were printing actual pointer addresses.  If you're just
 printing offsets relative to wherever the segment happens to be
 mapped, I don't care about that.

 Well, that just means that it's not an *obvious* security risk.

 I still like the idea of providing something comparable to
 MemoryContextStats, rather than creating a SQL interface.  The problem
 with a SQL interface is you can't interrogate it unless (1) you are not
 already inside a query and (2) the client is interactive and under your
 control.  Something you can call easily from gdb is likely to be much
 more useful in practice.

 Since the shared memory segment isn't changing at runtime, I don't see
 this as being a big problem.  It could possibly be an issue for
 dynamic shared memory segments, though.
Patch has been reviewed some time ago, extra ideas as well as
potential security risks discussed as well but no new version has been
sent, hence marking it as returned with feedback.
-- 
Michael


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

Re: [HACKERS] pg_shmem_allocations view

[Robert Haas]

On Fri, Aug 15, 2014 at 4:20 AM, Andres Freund and...@2ndquadrant.com wrote:
 On 2014-08-15 11:12:11 +0300, Marti Raudsepp wrote:
 Hi
 On Thu, May 8, 2014 at 4:28 PM, Andres Freund and...@2ndquadrant.com wrote:
  Ok. A new version of the patches implementing that are
  attached. Including a couple of small fixups and docs. The latter aren't
  extensive, but that doesn't seem to be warranted anyway.

 Is it really actually useful to expose the segment off(set) to users?
 Seems to me like unnecessary internal details leaking out.

 Yes. This is clearly developer oriented and I'd more than once wished I
 could see where some stray pointer is pointing to... That's not really
 possible without something like this.

Unfortunately, that information also has some security implications.
I'm sure someone trying to exploit any future stack-overrun
vulnerability will be very happy to have more rather than less
information about the layout of the process address space.

I fully agree with the idea of exposing the amount of free memory in
the shared memory segment (as discussed in other emails); that's
critical information.  But I think exposing address space layout
information is of much less general utility and, really, far too
risky.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


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

Re: [HACKERS] pg_shmem_allocations view

[Andres Freund]

On 2014-08-18 11:56:44 -0400, Robert Haas wrote:
 On Fri, Aug 15, 2014 at 4:20 AM, Andres Freund and...@2ndquadrant.com wrote:
  On 2014-08-15 11:12:11 +0300, Marti Raudsepp wrote:
  Hi
  On Thu, May 8, 2014 at 4:28 PM, Andres Freund and...@2ndquadrant.com 
  wrote:
   Ok. A new version of the patches implementing that are
   attached. Including a couple of small fixups and docs. The latter aren't
   extensive, but that doesn't seem to be warranted anyway.
 
  Is it really actually useful to expose the segment off(set) to users?
  Seems to me like unnecessary internal details leaking out.
 
  Yes. This is clearly developer oriented and I'd more than once wished I
  could see where some stray pointer is pointing to... That's not really
  possible without something like this.
 
 Unfortunately, that information also has some security implications.
 I'm sure someone trying to exploit any future stack-overrun
 vulnerability will be very happy to have more rather than less
 information about the layout of the process address space.

 I fully agree with the idea of exposing the amount of free memory in
 the shared memory segment (as discussed in other emails); that's
 critical information.  But I think exposing address space layout
 information is of much less general utility and, really, far too
 risky.

Meh. For one it's just the offsets, not the actual addresses. It's also
something you can relatively easily compute at home by looking at a
couple of settings everyone can see. For another, I'd be perfectly
content making this superuser only. And if somebody can execute queries
as superuser, address layout information really isn't needed anymore to
execute arbitrary code.

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

Re: [HACKERS] pg_shmem_allocations view

[Tom Lane]

Andres Freund and...@2ndquadrant.com writes:
 On 2014-08-18 11:56:44 -0400, Robert Haas wrote:
 I fully agree with the idea of exposing the amount of free memory in
 the shared memory segment (as discussed in other emails); that's
 critical information.  But I think exposing address space layout
 information is of much less general utility and, really, far too
 risky.

 Meh. For one it's just the offsets, not the actual addresses. It's also
 something you can relatively easily compute at home by looking at a
 couple of settings everyone can see. For another, I'd be perfectly
 content making this superuser only. And if somebody can execute queries
 as superuser, address layout information really isn't needed anymore to
 execute arbitrary code.

I agree that this has to be superuser-only if it's there at all.

Should we consider putting it into an extension rather than having
it in the core system?  That would offer some additional protection
for production systems, which really shouldn't have much need for
this IMO.

regards, tom lane


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

Re: [HACKERS] pg_shmem_allocations view

[Andres Freund]

On 2014-08-18 12:27:12 -0400, Tom Lane wrote:
 Andres Freund and...@2ndquadrant.com writes:
  On 2014-08-18 11:56:44 -0400, Robert Haas wrote:
  I fully agree with the idea of exposing the amount of free memory in
  the shared memory segment (as discussed in other emails); that's
  critical information.  But I think exposing address space layout
  information is of much less general utility and, really, far too
  risky.
 
  Meh. For one it's just the offsets, not the actual addresses. It's also
  something you can relatively easily compute at home by looking at a
  couple of settings everyone can see. For another, I'd be perfectly
  content making this superuser only. And if somebody can execute queries
  as superuser, address layout information really isn't needed anymore to
  execute arbitrary code.
 
 I agree that this has to be superuser-only if it's there at all.
 
 Should we consider putting it into an extension rather than having
 it in the core system?  That would offer some additional protection
 for production systems, which really shouldn't have much need for
 this IMO.

I'd considered that somewhere upthread and decided that it'd require
exposing to much internals from shmem.c/dsm.c without a corresponding
benefit.

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

Re: [HACKERS] pg_shmem_allocations view

[Tom Lane]

Andres Freund and...@2ndquadrant.com writes:
 On 2014-08-18 12:27:12 -0400, Tom Lane wrote:
 Should we consider putting it into an extension rather than having
 it in the core system?  That would offer some additional protection
 for production systems, which really shouldn't have much need for
 this IMO.

 I'd considered that somewhere upthread and decided that it'd require
 exposing to much internals from shmem.c/dsm.c without a corresponding
 benefit.

Well, we could have the implementation code in those modules but not
provide any SQL-level access to it without installing an extension.
The only extra thing visible in the .h files would be a function or two.

regards, tom lane


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

Re: [HACKERS] pg_shmem_allocations view

[Andres Freund]

On 2014-08-18 12:33:44 -0400, Tom Lane wrote:
 Andres Freund and...@2ndquadrant.com writes:
  On 2014-08-18 12:27:12 -0400, Tom Lane wrote:
  Should we consider putting it into an extension rather than having
  it in the core system?  That would offer some additional protection
  for production systems, which really shouldn't have much need for
  this IMO.
 
  I'd considered that somewhere upthread and decided that it'd require
  exposing to much internals from shmem.c/dsm.c without a corresponding
  benefit.
 
 Well, we could have the implementation code in those modules but not
 provide any SQL-level access to it without installing an extension.
 The only extra thing visible in the .h files would be a function or two.

That'd require wrapper functions in the extension afaics. Not that that
is prohibitive, but a bit inconvenient. At least I don't see another way
to create a sql function referring to a builtin C implementation. I
don't think PG_FUNCTION_INFO_V1() can reliably made work. We could have
the underlying function in pg_proc, but not create the view...

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

Re: [HACKERS] pg_shmem_allocations view

[Tom Lane]

Andres Freund and...@2ndquadrant.com writes:
 On 2014-08-18 12:33:44 -0400, Tom Lane wrote:
 Well, we could have the implementation code in those modules but not
 provide any SQL-level access to it without installing an extension.
 The only extra thing visible in the .h files would be a function or two.

 That'd require wrapper functions in the extension afaics.

Sure.  I'd want to put as much of the logic as possible in the extension,
anyway.

regards, tom lane


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

Re: [HACKERS] pg_shmem_allocations view

[Robert Haas]

On Mon, Aug 18, 2014 at 12:00 PM, Andres Freund and...@2ndquadrant.com wrote:
 Unfortunately, that information also has some security implications.
 I'm sure someone trying to exploit any future stack-overrun
 vulnerability will be very happy to have more rather than less
 information about the layout of the process address space.


 Meh. For one it's just the offsets, not the actual addresses. It's also
 something you can relatively easily compute at home by looking at a
 couple of settings everyone can see. For another, I'd be perfectly
 content making this superuser only. And if somebody can execute queries
 as superuser, address layout information really isn't needed anymore to
 execute arbitrary code.

I'm just not sure it should be in there at all.  If we punt this off
into an extension, it won't be available in a lot of situations where
it's really needed.  But although the basic functionality would have
been quite useful to me on any number of occasions, I can't recall a
single occasion upon which I would have cared about the offset at all.
I wouldn't mind having a MemoryContextStats()-type function that could
be used to print this information out by attaching to the backend with
gdb, but the utility of exposing it at the SQL level seems very
marginal to me.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


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

Re: [HACKERS] pg_shmem_allocations view

[Andres Freund]

On 2014-08-18 12:41:58 -0400, Robert Haas wrote:
 On Mon, Aug 18, 2014 at 12:00 PM, Andres Freund and...@2ndquadrant.com 
 wrote:
  Unfortunately, that information also has some security implications.
  I'm sure someone trying to exploit any future stack-overrun
  vulnerability will be very happy to have more rather than less
  information about the layout of the process address space.
 
 
  Meh. For one it's just the offsets, not the actual addresses. It's also
  something you can relatively easily compute at home by looking at a
  couple of settings everyone can see. For another, I'd be perfectly
  content making this superuser only. And if somebody can execute queries
  as superuser, address layout information really isn't needed anymore to
  execute arbitrary code.
 
 I'm just not sure it should be in there at all.

You realize that you can pretty much recompute the offsets from the
sizes of the individual allocations anyway? Yes, you need to add some
rounding, but that's about it. We could randomize the returned elements,
but that'd be rather annoying because it'd loose information.

  If we punt this off
 into an extension, it won't be available in a lot of situations where
 it's really needed.  But although the basic functionality would have
 been quite useful to me on any number of occasions, I can't recall a
 single occasion upon which I would have cared about the offset at all.
 I wouldn't mind having a MemoryContextStats()-type function that could
 be used to print this information out by attaching to the backend with
 gdb, but the utility of exposing it at the SQL level seems very
 marginal to me.

I'd use for it in the past when trying to figure out what some pointer
pointed to. It's easy enough to figure out that it's in the main shared
memory segment, but after that it get's rather hard. And even if you
don't count that, it gives a sensible order to the returned rows from
the SRF.

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

Re: [HACKERS] pg_shmem_allocations view

[Robert Haas]

On Mon, Aug 18, 2014 at 12:46 PM, Andres Freund and...@2ndquadrant.com wrote:
 On 2014-08-18 12:41:58 -0400, Robert Haas wrote:
 On Mon, Aug 18, 2014 at 12:00 PM, Andres Freund and...@2ndquadrant.com 
 wrote:
  Unfortunately, that information also has some security implications.
  I'm sure someone trying to exploit any future stack-overrun
  vulnerability will be very happy to have more rather than less
  information about the layout of the process address space.
 
 
  Meh. For one it's just the offsets, not the actual addresses. It's also
  something you can relatively easily compute at home by looking at a
  couple of settings everyone can see. For another, I'd be perfectly
  content making this superuser only. And if somebody can execute queries
  as superuser, address layout information really isn't needed anymore to
  execute arbitrary code.

 I'm just not sure it should be in there at all.

 You realize that you can pretty much recompute the offsets from the
 sizes of the individual allocations anyway?

Sure, if you know the segment base.  Do you?

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


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

Re: [HACKERS] pg_shmem_allocations view

[Andres Freund]

On 2014-08-18 12:50:27 -0400, Robert Haas wrote:
 On Mon, Aug 18, 2014 at 12:46 PM, Andres Freund and...@2ndquadrant.com 
 wrote:
  You realize that you can pretty much recompute the offsets from the
  sizes of the individual allocations anyway?
 
 Sure, if you know the segment base.  Do you?

Err? The offset doesn't give you the base either?

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

Re: [HACKERS] pg_shmem_allocations view

[Tom Lane]

Robert Haas robertmh...@gmail.com writes:
 I wouldn't mind having a MemoryContextStats()-type function that could
 be used to print this information out by attaching to the backend with
 gdb, but the utility of exposing it at the SQL level seems very
 marginal to me.

+1 for doing it like that.

regards, tom lane


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

Re: [HACKERS] pg_shmem_allocations view

[Robert Haas]

On Mon, Aug 18, 2014 at 12:51 PM, Andres Freund and...@2ndquadrant.com wrote:
 On 2014-08-18 12:50:27 -0400, Robert Haas wrote:
 On Mon, Aug 18, 2014 at 12:46 PM, Andres Freund and...@2ndquadrant.com 
 wrote:
  You realize that you can pretty much recompute the offsets from the
  sizes of the individual allocations anyway?

 Sure, if you know the segment base.  Do you?

 Err? The offset doesn't give you the base either?

Oh!

I thought you were printing actual pointer addresses.  If you're just
printing offsets relative to wherever the segment happens to be
mapped, I don't care about that.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


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

Re: [HACKERS] pg_shmem_allocations view

[Tom Lane]

Robert Haas robertmh...@gmail.com writes:
 I thought you were printing actual pointer addresses.  If you're just
 printing offsets relative to wherever the segment happens to be
 mapped, I don't care about that.

Well, that just means that it's not an *obvious* security risk.

I still like the idea of providing something comparable to
MemoryContextStats, rather than creating a SQL interface.  The problem
with a SQL interface is you can't interrogate it unless (1) you are not
already inside a query and (2) the client is interactive and under your
control.  Something you can call easily from gdb is likely to be much
more useful in practice.

regards, tom lane


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

Re: [HACKERS] pg_shmem_allocations view

[Andres Freund]

On 2014-08-18 13:27:07 -0400, Tom Lane wrote:
 I still like the idea of providing something comparable to
 MemoryContextStats, rather than creating a SQL interface.  The problem
 with a SQL interface is you can't interrogate it unless (1) you are not
 already inside a query and (2) the client is interactive and under your
 control.  Something you can call easily from gdb is likely to be much
 more useful in practice.

That might be true in some cases, but in many cases interfaces that can
only be used via gdb *SUCK*. A good reason to use the interface proposed
here is to investigate which extensions are allocating how much shared
memory. A pretty normal question to ask as a sysadmin/DBA. And DBA type
of stuff should never have to involve gdb.

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

Re: [HACKERS] pg_shmem_allocations view

[Robert Haas]

On Mon, Aug 18, 2014 at 1:27 PM, Tom Lane t...@sss.pgh.pa.us wrote:
 Robert Haas robertmh...@gmail.com writes:
 I thought you were printing actual pointer addresses.  If you're just
 printing offsets relative to wherever the segment happens to be
 mapped, I don't care about that.

 Well, that just means that it's not an *obvious* security risk.

 I still like the idea of providing something comparable to
 MemoryContextStats, rather than creating a SQL interface.  The problem
 with a SQL interface is you can't interrogate it unless (1) you are not
 already inside a query and (2) the client is interactive and under your
 control.  Something you can call easily from gdb is likely to be much
 more useful in practice.

Since the shared memory segment isn't changing at runtime, I don't see
this as being a big problem.  It could possibly be an issue for
dynamic shared memory segments, though.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


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

Re: [HACKERS] pg_shmem_allocations view

[Marti Raudsepp]

Hi

On Thu, May 8, 2014 at 4:28 PM, Andres Freund and...@2ndquadrant.com wrote:
 Ok. A new version of the patches implementing that are
 attached. Including a couple of small fixups and docs. The latter aren't
 extensive, but that doesn't seem to be warranted anyway.

Is it really actually useful to expose the segment off(set) to users?
Seems to me like unnecessary internal details leaking out.

Regards,
Marti


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

Re: [HACKERS] pg_shmem_allocations view

[Andres Freund]

On 2014-08-15 11:12:11 +0300, Marti Raudsepp wrote:
 Hi
 
 On Thu, May 8, 2014 at 4:28 PM, Andres Freund and...@2ndquadrant.com wrote:
  Ok. A new version of the patches implementing that are
  attached. Including a couple of small fixups and docs. The latter aren't
  extensive, but that doesn't seem to be warranted anyway.
 
 Is it really actually useful to expose the segment off(set) to users?
 Seems to me like unnecessary internal details leaking out.

Yes. This is clearly developer oriented and I'd more than once wished I
could see where some stray pointer is pointing to... That's not really
possible without something like this.

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

Re: [HACKERS] pg_shmem_allocations view

[Andres Freund]

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#literalNULL/ (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

Re: [HACKERS] pg_shmem_allocations view

[Michael Paquier]

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#literalNULL/ (in the docs as this refers to a value)
- 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.
- off should be renamed to offset for pg_get_shmem_allocations.
- Is it really worth showing unused shared memory? I'd rather rip out
the last portion of pg_get_shmem_allocations.
- 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.

Regards,
-- 
Michael


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

Re: [HACKERS] pg_shmem_allocations view

[Michael Paquier]

On Thu, May 8, 2014 at 10:28 PM, Andres Freund and...@2ndquadrant.com wrote:
 Well, we have to live with it for now :)
I just had a look at the first patch and got some comments:
1) Instead of using an assertion here, wouldn't it be better to error
out if name is NULL, and truncate the name if it is longer than
SHMEM_INDEX_KEYSIZE - 1 (including '\0')?
scanstr in scansup.c?
Assert(IsUnderPostmaster);
+   Assert(name != NULL  strlen(name)  0 
+  strlen(name)  SHMEM_INDEX_KEYSIZE - 1);
2) The addition of a field to track the size of a dsm should be
explicitly mentioned, this is useful for the 2nd patch.
3) The refactoring done in dsm_create to find an unused slot should be
done as a separate patch for clarity.
4) Using '\0' here would be more adapted:
+   item-name[SHMEM_INDEX_KEYSIZE - 1] = 0;

Regards,
-- 
Michael


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

Re: [HACKERS] pg_shmem_allocations view

[Abhijit Menon-Sen]

At 2014-05-08 15:28:22 +0200, and...@2ndquadrant.com wrote:

   Hm. Not sure what you're ACKing here ;).
  
  The idea of giving the unallocated memory a NULL key.
 
 Ok. A new version of the patches implementing that are attached.
 Including a couple of small fixups and docs. The latter aren't
 extensive, but that doesn't seem to be warranted anyway.

I realise now that this email didn't actually have an attachment. Could
you please repost the latest version of this patch?

Thanks.

-- Abhijit


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

Re: [HACKERS] pg_shmem_allocations view

[Robert Haas]

On Mon, Jul 14, 2014 at 6:20 AM, Abhijit Menon-Sen a...@2ndquadrant.com wrote:
 At 2014-05-08 15:28:22 +0200, and...@2ndquadrant.com wrote:
   Hm. Not sure what you're ACKing here ;).
 
  The idea of giving the unallocated memory a NULL key.

 Ok. A new version of the patches implementing that are attached.
 Including a couple of small fixups and docs. The latter aren't
 extensive, but that doesn't seem to be warranted anyway.

 I realise now that this email didn't actually have an attachment. Could
 you please repost the latest version of this patch?

That's odd.  I received two attachments with that email.  Of course, I
was copied directly, but why would the archives have lost the
attachments?

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


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

Re: [HACKERS] pg_shmem_allocations view

[Alvaro Herrera]

Robert Haas wrote:
 On Mon, Jul 14, 2014 at 6:20 AM, Abhijit Menon-Sen a...@2ndquadrant.com 
 wrote:
  At 2014-05-08 15:28:22 +0200, and...@2ndquadrant.com wrote:
Hm. Not sure what you're ACKing here ;).
  
   The idea of giving the unallocated memory a NULL key.
 
  Ok. A new version of the patches implementing that are attached.
  Including a couple of small fixups and docs. The latter aren't
  extensive, but that doesn't seem to be warranted anyway.
 
  I realise now that this email didn't actually have an attachment. Could
  you please repost the latest version of this patch?
 
 That's odd.  I received two attachments with that email.  Of course, I
 was copied directly, but why would the archives have lost the
 attachments?

The attachments are there on the archives, and also on my mbox -- and
unlike Robert, I was not copied.  I think this is a problem on Abhijit's
end.

-- 
Álvaro Herrerahttp://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

Re: [HACKERS] pg_shmem_allocations view

[Abhijit Menon-Sen]

At 2014-07-14 16:48:09 -0400, alvhe...@2ndquadrant.com wrote:

 The attachments are there on the archives, and also on my mbox -- and
 unlike Robert, I was not copied.  I think this is a problem on
 Abhijit's end.

Yes, it is. I apologise for the noise.

-- Abhijit


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

Re: [HACKERS] pg_shmem_allocations view

[Robert Haas]

On Wed, May 7, 2014 at 5:54 PM, Andres Freund and...@2ndquadrant.com wrote:
 On 2014-05-07 17:48:15 -0400, Robert Haas wrote:
 On Tue, May 6, 2014 at 6:09 PM, Andres Freund and...@2ndquadrant.com wrote:
  I guess I'd vote for
  ditching the allocated column completely and outputting the memory
  allocated without ShmemIndex using some fixed tag (like ShmemIndex
  or Bootstrap or Overhead or something).
 
  My way feels slightly cleaner, but I'd be ok with that as well. There's
  no possible conflicts with an actual segment... In your variant the
  unallocated/slop memory would continue to have a NULL key?

 Yeah, that seems all right.

 Hm. Not sure what you're ACKing here ;).

The idea of giving the unallocated memory a NULL key.

 One way to avoid conflict with an actual segment would be to add an
 after-the-fact entry into ShmemIndex representing the amount of memory
 that was used to bootstrap it.

 There's lots of allocations from shmem that cannot be associated with
 any index entry though. Not just ShmemIndex's own entry. Most
 prominently most of the memory used for SharedBufHash isn't actually
 associated with the Shared Buffer Lookup Table entry - imo a
 dynahash.c defficiency.

Hmm, I don't know what to do about that.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


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

Re: [HACKERS] pg_shmem_allocations view

[Andres Freund]

On 2014-05-08 07:58:34 -0400, Robert Haas wrote:
 On Wed, May 7, 2014 at 5:54 PM, Andres Freund and...@2ndquadrant.com wrote:
  Hm. Not sure what you're ACKing here ;).
 
 The idea of giving the unallocated memory a NULL key.

Ok. A new version of the patches implementing that are
attached. Including a couple of small fixups and docs. The latter aren't
extensive, but that doesn't seem to be warranted anyway.

  There's lots of allocations from shmem that cannot be associated with
  any index entry though. Not just ShmemIndex's own entry. Most
  prominently most of the memory used for SharedBufHash isn't actually
  associated with the Shared Buffer Lookup Table entry - imo a
  dynahash.c defficiency.

 Hmm, I don't know what to do about that.

Well, we have to live with it for now :)

Greetings,

Andres Freund

-- 
 Andres Freund http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training  Services
From c219c03a173fef962c1caba9f016d5d87448fd8f Mon Sep 17 00:00:00 2001
From: Andres Freund and...@anarazel.de
Date: Tue, 6 May 2014 19:42:36 +0200
Subject: [PATCH 1/4] Associate names to created dynamic shared memory
 segments.

At some later point we want to add a view show all allocated dynamic
shared memory segments so admins can understand resource usage. To
avoid breaking the API in 9.5 add the necessary name now.
---
 contrib/test_shm_mq/setup.c   |  2 +-
 src/backend/storage/ipc/dsm.c | 60 ++-
 src/include/storage/dsm.h |  2 +-
 3 files changed, 39 insertions(+), 25 deletions(-)

diff --git a/contrib/test_shm_mq/setup.c b/contrib/test_shm_mq/setup.c
index 572cf88..897c47b 100644
--- a/contrib/test_shm_mq/setup.c
+++ b/contrib/test_shm_mq/setup.c
@@ -125,7 +125,7 @@ setup_dynamic_shared_memory(int64 queue_size, int nworkers,
 	segsize = shm_toc_estimate(e);
 
 	/* Create the shared memory segment and establish a table of contents. */
-	seg = dsm_create(shm_toc_estimate(e));
+	seg = dsm_create(test_shm_mq, shm_toc_estimate(e));
 	toc = shm_toc_create(PG_TEST_SHM_MQ_MAGIC, dsm_segment_address(seg),
 		 segsize);
 
diff --git a/src/backend/storage/ipc/dsm.c b/src/backend/storage/ipc/dsm.c
index a5c0084..c8fdf6e 100644
--- a/src/backend/storage/ipc/dsm.c
+++ b/src/backend/storage/ipc/dsm.c
@@ -80,8 +80,10 @@ struct dsm_segment
 /* Shared-memory state for a dynamic shared memory segment. */
 typedef struct dsm_control_item
 {
-	dsm_handle	handle;
+	dsm_handle	handle;			/* segment identifier */
 	uint32		refcnt;			/* 2+ = active, 1 = moribund, 0 = gone */
+	Size		size;			/* current size */
+	char		name[SHMEM_INDEX_KEYSIZE]; /* informational name */
 } dsm_control_item;
 
 /* Layout of the dynamic shared memory control segment. */
@@ -454,14 +456,16 @@ dsm_set_control_handle(dsm_handle h)
  * Create a new dynamic shared memory segment.
  */
 dsm_segment *
-dsm_create(Size size)
+dsm_create(const char *name, Size size)
 {
 	dsm_segment *seg = dsm_create_descriptor();
-	uint32		i;
-	uint32		nitems;
+	dsm_control_item *item;
+	uint32		slot;
 
 	/* Unsafe in postmaster (and pointless in a stand-alone backend). */
 	Assert(IsUnderPostmaster);
+	Assert(name != NULL  strlen(name)  0 
+		   strlen(name)  SHMEM_INDEX_KEYSIZE - 1);
 
 	if (!dsm_init_done)
 		dsm_backend_startup();
@@ -479,33 +483,39 @@ dsm_create(Size size)
 	/* Lock the control segment so we can register the new segment. */
 	LWLockAcquire(DynamicSharedMemoryControlLock, LW_EXCLUSIVE);
 
-	/* Search the control segment for an unused slot. */
-	nitems = dsm_control-nitems;
-	for (i = 0; i  nitems; ++i)
+	/*
+	 * Search the control segment for an unused slot that's previously been
+	 * used. If we don't find one initialize a new one if there's still space.
+	 */
+	for (slot = 0; slot  dsm_control-nitems; ++slot)
 	{
-		if (dsm_control-item[i].refcnt == 0)
-		{
-			dsm_control-item[i].handle = seg-handle;
-			/* refcnt of 1 triggers destruction, so start at 2 */
-			dsm_control-item[i].refcnt = 2;
-			seg-control_slot = i;
-			LWLockRelease(DynamicSharedMemoryControlLock);
-			return seg;
-		}
+		if (dsm_control-item[slot].refcnt == 0)
+			break;
 	}
 
-	/* Verify that we can support an additional mapping. */
-	if (nitems = dsm_control-maxitems)
+	/* Verify that we can support the mapping. */
+	if (slot = dsm_control-maxitems)
 		ereport(ERROR,
 (errcode(ERRCODE_INSUFFICIENT_RESOURCES),
  errmsg(too many dynamic shared memory segments)));
 
-	/* Enter the handle into a new array slot. */
-	dsm_control-item[nitems].handle = seg-handle;
+	item = dsm_control-item[slot];
+	item-handle = seg-handle;
 	/* refcnt of 1 triggers destruction, so start at 2 */
-	dsm_control-item[nitems].refcnt = 2;
-	seg-control_slot = nitems;
-	dsm_control-nitems++;
+	item-refcnt = 2;
+	item-size = size;
+	strncpy(item-name, name, SHMEM_INDEX_KEYSIZE);
+	item-name[SHMEM_INDEX_KEYSIZE - 1] = 0;
+
+	seg-control_slot = slot;
+
+	/*
+	 * Increase number of initilized slots if we didn't reuse a 

Re: [HACKERS] pg_shmem_allocations view

[Robert Haas]

On Tue, May 6, 2014 at 6:09 PM, Andres Freund and...@2ndquadrant.com wrote:
 I guess I'd vote for
 ditching the allocated column completely and outputting the memory
 allocated without ShmemIndex using some fixed tag (like ShmemIndex
 or Bootstrap or Overhead or something).

 My way feels slightly cleaner, but I'd be ok with that as well. There's
 no possible conflicts with an actual segment... In your variant the
 unallocated/slop memory would continue to have a NULL key?

Yeah, that seems all right.

One way to avoid conflict with an actual segment would be to add an
after-the-fact entry into ShmemIndex representing the amount of memory
that was used to bootstrap it.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


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

Re: [HACKERS] pg_shmem_allocations view

[Andres Freund]

On 2014-05-07 17:48:15 -0400, Robert Haas wrote:
 On Tue, May 6, 2014 at 6:09 PM, Andres Freund and...@2ndquadrant.com wrote:
  I guess I'd vote for
  ditching the allocated column completely and outputting the memory
  allocated without ShmemIndex using some fixed tag (like ShmemIndex
  or Bootstrap or Overhead or something).
 
  My way feels slightly cleaner, but I'd be ok with that as well. There's
  no possible conflicts with an actual segment... In your variant the
  unallocated/slop memory would continue to have a NULL key?
 
 Yeah, that seems all right.

Hm. Not sure what you're ACKing here ;).

 One way to avoid conflict with an actual segment would be to add an
 after-the-fact entry into ShmemIndex representing the amount of memory
 that was used to bootstrap it.

There's lots of allocations from shmem that cannot be associated with
any index entry though. Not just ShmemIndex's own entry. Most
prominently most of the memory used for SharedBufHash isn't actually
associated with the Shared Buffer Lookup Table entry - imo a
dynahash.c defficiency.

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

Re: [HACKERS] pg_shmem_allocations view

[Simon Riggs]

On 5 May 2014 21:54, Robert Haas robertmh...@gmail.com wrote:
 On Mon, May 5, 2014 at 3:09 PM, Tom Lane t...@sss.pgh.pa.us wrote:
 Robert Haas robertmh...@gmail.com writes:
 On Sun, May 4, 2014 at 7:50 AM, Andres Freund and...@2ndquadrant.com 
 wrote:
 Thinking about this, I think it was a mistake to not add a 'name' field
 to dynamic shared memory's dsm_control_item.

 Well, right now a dsm_control_item is 8 bytes.  If we add a name field
 of our usual 64 bytes, they'll each be 9 times bigger.

 And the controlled shared segment is likely to be how big exactly?  It's
 probably not even possible for it to be smaller than a page size, 4K or
 so depending on the OS.  I agree with Andres that a name would be a good
 idea; complaining about the space needed to hold it is penny-wise and
 pound-foolish.

 The control segment is sized to support a number of dynamic shared
 memory segments not exceeding 64 + 2 *MaxBackends.  With default
 settings, that currently works out to 288 segments, or 2306 bytes.
 So, adding a 64-byte name to each of those structures would increase
 the size from 2k to about 20k.

 So, sure, that's not a lot of memory.  But I'm still not convinced
 that's it's very useful.  What I think is going to happen is that (1)
 most people won't be used dynamic shared memory at all, so they won't
 have any use for this; (2) those people who do run an extension that
 uses dynamic shared memory will most likely only be running one such
 extension, so they won't need a name to know what the segments are
 being used for; and (3) if and when we eventually get parallel query,
 dynamic shared memory segments will be widely used, but a bunch of
 segments that are all named parallel_query or parallel_query.$PID
 isn't going to be too informative.

Not sure your arguments hold any water.

Most people don't use most features... and so we're not allowed
features that can be debugged?
How do you know people will only use one extension that uses dshmem?
Why would we call multiple segments the same thing??

If names are a problem, lets give them numbers. Seems a minor point.
Perhaps we can allocate space for names dynamically??

Not being able to tell segments apart from each other is just daft, if
we are trying to supply bug free software for the world to use.

-- 
 Simon Riggs   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

Re: [HACKERS] pg_shmem_allocations view

[Andres Freund]

On 2014-05-05 23:20:43 -0400, Robert Haas wrote:
 On Mon, May 5, 2014 at 6:54 PM, Tom Lane t...@sss.pgh.pa.us wrote:
  I'm not confident that it'll be useful either.  But I am confident that
  if we don't put it in now, and decide we want it later, there will be
  complaints when we change the API.  Better to have an ignored parameter
  than no parameter.
 
 I'm generally skeptical of that philosophy.  If we put in an ignored
 parameter, people may pass pointers to NULL or to garbage or to an
 overly-long string, and they won't know it's broken until we make it
 do something; at which point their code will begin to fail without
 warning.

If it were a complex change, maybe. But I don't think that's likely
here.
Assert(name != NULL  strlen(name)  0  strlen(name)  NAMEDATALEN);
should perfectly do the trick.

 If we're going to do anything at all here for 9.4, I recommend
 ignoring the fact we're in feature freeze and going whole hog: add the
 name, add the monitoring view, and add the monitoring view for the
 main shared memory segment just for good measure.

We can do that as well. If there's agreement on that path I'll update
the patch to also show dynamic statements.

 Anyone who expects PostgreSQL's C API to be
 completely stable is going to be regularly disappointed, as most
 recently demonstrated by the Enormous Header Churn of the 9.3 era.  I
 don't particularly mind being the cause of further disappointment; as
 long as the breakage is obvious rather than subtle, the fix usually
 takes about 10 minutes.

Didn't you complain rather loudly about that change?

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

Re: [HACKERS] pg_shmem_allocations view

[Robert Haas]

On Tue, May 6, 2014 at 7:45 AM, Simon Riggs si...@2ndquadrant.com wrote:
 The control segment is sized to support a number of dynamic shared
 memory segments not exceeding 64 + 2 *MaxBackends.  With default
 settings, that currently works out to 288 segments, or 2306 bytes.
 So, adding a 64-byte name to each of those structures would increase
 the size from 2k to about 20k.

 So, sure, that's not a lot of memory.  But I'm still not convinced
 that's it's very useful.  What I think is going to happen is that (1)
 most people won't be used dynamic shared memory at all, so they won't
 have any use for this; (2) those people who do run an extension that
 uses dynamic shared memory will most likely only be running one such
 extension, so they won't need a name to know what the segments are
 being used for; and (3) if and when we eventually get parallel query,
 dynamic shared memory segments will be widely used, but a bunch of
 segments that are all named parallel_query or parallel_query.$PID
 isn't going to be too informative.

 Not sure your arguments hold any water.

I'm not, either.

 Most people don't use most features... and so we're not allowed
 features that can be debugged?

I certainly didn't say that.

 How do you know people will only use one extension that uses dshmem?

I don't.  If they do, that's a good argument for adding this.

 Why would we call multiple segments the same thing??

It's not clear to me how someone is going to intelligently name
multiple segments used by the same extension.  Maybe they'll give them
all the same name.  Maybe they'll name them all extension_name.pid.
More than likely, different extensions will use different conventions.
 :-(

It might be sensible to add a creator PID field to the DSM control
items.  Of course, that PID might have exited, but it could still
possibly be useful for debugging purposes.

 If names are a problem, lets give them numbers. Seems a minor point.
 Perhaps we can allocate space for names dynamically??

A static buffer, as proposed by Andres, seems a lot simper.

 Not being able to tell segments apart from each other is just daft, if
 we are trying to supply bug free software for the world to use.

I can see I'm losing this argument.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


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

Re: [HACKERS] pg_shmem_allocations view

[Heikki Linnakangas]

On 05/06/2014 02:59 PM, Robert Haas wrote:

Why would we call multiple segments the same thing??

It's not clear to me how someone is going to intelligently name
multiple segments used by the same extension.  Maybe they'll give them
all the same name.  Maybe they'll name them all extension_name.pid.
More than likely, different extensions will use different conventions.
  :-(


That seems sensible to me. The best scheme will depend on how the 
segments are used. Best to leave it to the extension author.


- Heikki


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

Re: [HACKERS] pg_shmem_allocations view

[Simon Riggs]

On 6 May 2014 13:06, Heikki Linnakangas hlinnakan...@vmware.com wrote:

 The best scheme will depend on how the segments
 are used. Best to leave it to the extension author.

+1

-- 
 Simon Riggs   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

Re: [HACKERS] pg_shmem_allocations view

[Andres Freund]

Hi,

On 2014-05-06 13:56:41 +0200, Andres Freund wrote:
 On 2014-05-05 23:20:43 -0400, Robert Haas wrote:
  On Mon, May 5, 2014 at 6:54 PM, Tom Lane t...@sss.pgh.pa.us wrote:
   I'm not confident that it'll be useful either.  But I am confident that
   if we don't put it in now, and decide we want it later, there will be
   complaints when we change the API.  Better to have an ignored parameter
   than no parameter.
 
  I'm generally skeptical of that philosophy.  If we put in an ignored
  parameter, people may pass pointers to NULL or to garbage or to an
  overly-long string, and they won't know it's broken until we make it
  do something; at which point their code will begin to fail without
  warning.

 If it were a complex change, maybe. But I don't think that's likely
 here.
 Assert(name != NULL  strlen(name)  0  strlen(name)  NAMEDATALEN);
 should perfectly do the trick.

Attached are two patches:
a) Patch addin a 'name' parameter to dsm_create(). I think we should
   apply this to 9.4.
b) pg_dynamic_shmem_allocations and pg_static_shmem_allocations
   views. The previous version didn't include dsm support and didn't
   take the required lock.

I am not so sure whether b) should be applied together with a) in 9.4,
but I'd be happy enough to add docs if people agree with the naming.

FWIW, I like dsm_create()'s internals more after this patch...

postgres=# \d pg_dynamic_shmem_allocations
View pg_catalog.pg_dynamic_shmem_allocations
 Column |  Type  | Modifiers
++---
 handle | bigint |
 name   | text   |
 size   | bigint |
 refcnt | bigint |

postgres=# \d pg_static_shmem_allocations
View pg_catalog.pg_static_shmem_allocations
  Column   |  Type   | Modifiers
---+-+---
 key   | text|
 off   | bigint  |
 size  | bigint  |
 allocated | boolean |

postgres=# SELECT * FROM pg_dynamic_shmem_allocations;
   handle   |name | size  | refcnt
+-+---+
 1120921036 | test_shm_mq | 65656 |  1
(1 row)

postgres=# SELECT * FROM pg_static_shmem_allocations ORDER BY key NULLS FIRST;
 key |off |size| allocated
-+++---
 | 605024 |1727776 | f
 ||   34844752 | t
 Async Ctl   | 539168 |  65856 | t
 Async Queue Control | 537784 |   1384 | t
 AutoVacuum Data | 533576 |224 | t
 Backend Activity Buffer | 2217099552 | 114688 | t
 Backend Application Name Buffer | 2217085216 |   7168 | t
 Backend Client Host Name Buffer | 2217092384 |   7168 | t
 Backend Status Array| 2217061024 |  24192 | t
 Background Worker Data  | 2217214256 |   1992 | t
 BTree Vacuum State  | 535768 |   1356 | t
 Buffer Blocks   |   51365312 | 2147483648 | t
 Buffer Descriptors  |   34588096 |   16777216 | t
 Buffer Strategy Status  | 2213546176 | 32 | t
 Checkpointer Data   | 2217290656 |5242920 | t
 CLOG Ctl|   33601152 | 525312 | t
 Control File|   16796384 |240 | t
 Fast Path Strong Relation Lock Data | 2214767072 |   4100 | t
 FinishedSerializableTransactions| 2216841952 | 16 | t
 LOCK hash   | 2213546208 |   2160 | t
 MultiXactMember Ctl |   34455488 | 131648 | t
 MultiXactOffset Ctl |   34389632 |  65856 | t
 OldSerXidControlData| 2216973632 | 16 | t
 OldSerXid SLRU Ctl  | 2216841984 | 131648 | t
 PMSignalState   | 2217285400 |940 | t
 PREDICATELOCK hash  | 2215182944 |   2160 | t
 PREDICATELOCKTARGET hash| 2214771176 |   2160 | t
 PredXactList| 2216348384 | 88 | t
 Prepared Transaction Table  | 2217214240 | 16 | t
 Proc Array  | 2217060536 |488 | t
 Proc Header | 2216973648 | 88 | t
 PROCLOCK hash   | 2214183264 |   2160 | t
 ProcSignalSlots | 2217286344 |   4284 | t
 RWConflictPool  | 2216573120 | 24 | t
 SERIALIZABLEXID hash| 2216518720 |   2160 | t
 Shared Buffer Lookup Table  | 2198848960 |  16496 | t
 Shared MultiXact State  |   34587136 |936 | t
 shmInvalBuffer  | 2217216256 |  69144 | t
 SUBTRANS Ctl|   34126464 | 263168 | t
 Sync Scan Locations List| 537128 |656 | t
 Wal Receiver Ctl| 534576 | 

Re: [HACKERS] pg_shmem_allocations view

[Tom Lane]

Andres Freund and...@2ndquadrant.com writes:
 Attached are two patches:
 a) Patch addin a 'name' parameter to dsm_create(). I think we should
apply this to 9.4.
 b) pg_dynamic_shmem_allocations and pg_static_shmem_allocations
views. The previous version didn't include dsm support and didn't
take the required lock.

 I am not so sure whether b) should be applied together with a) in 9.4,
 but I'd be happy enough to add docs if people agree with the naming.

FWIW, I vote for fixing (a) now but holding (b) for 9.5.

regards, tom lane


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

Re: [HACKERS] pg_shmem_allocations view

[Robert Haas]

On Tue, May 6, 2014 at 2:34 PM, Tom Lane t...@sss.pgh.pa.us wrote:
 Andres Freund and...@2ndquadrant.com writes:
 Attached are two patches:
 a) Patch addin a 'name' parameter to dsm_create(). I think we should
apply this to 9.4.
 b) pg_dynamic_shmem_allocations and pg_static_shmem_allocations
views. The previous version didn't include dsm support and didn't
take the required lock.

 I am not so sure whether b) should be applied together with a) in 9.4,
 but I'd be happy enough to add docs if people agree with the naming.

 FWIW, I vote for fixing (a) now but holding (b) for 9.5.

I guess I'll vote for applying both.  I don't see a lot of risk, and I
think doing one with out the other is somewhat pointless.

Regarding patch 0002, I don't think we're using the term static
shmem anywhere else, so I vote for dropping the word static, so that
we have pg_get_shmem_allocations() and
pg_get_dynamic_shmem_allocations().  Also, I think using the
allocated column is pretty weird.  There are always exactly two
entries with allocated = false, one of which is for actual free memory
and the other of which is for memory that actually IS allocated but
without using ShmemIndex (maybe the latter was supposed to have
allocated = true but still key = null?).  I guess I'd vote for
ditching the allocated column completely and outputting the memory
allocated without ShmemIndex using some fixed tag (like ShmemIndex
or Bootstrap or Overhead or something).

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


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

Re: [HACKERS] pg_shmem_allocations view

[Tom Lane]

Robert Haas robertmh...@gmail.com writes:
 On Tue, May 6, 2014 at 2:34 PM, Tom Lane t...@sss.pgh.pa.us wrote:
 FWIW, I vote for fixing (a) now but holding (b) for 9.5.

 I guess I'll vote for applying both.  I don't see a lot of risk, and I
 think doing one with out the other is somewhat pointless.

The difference is that there's not consensus about the details of the
views ... as borne out by your next paragraph.

Now admittedly, we could always redefine the views in 9.5, but
I'd rather not be doing this sort of thing in haste.  Something
as user-visible as a system view really ought to have baked awhile
before we ship it.  Patch (a) is merely institutionalizing the
expectation that DSM segments should have names, which is a much
lower-risk bet.

regards, tom lane


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

Re: [HACKERS] pg_shmem_allocations view

[Simon Riggs]

On 6 May 2014 20:44, Tom Lane t...@sss.pgh.pa.us wrote:
 Robert Haas robertmh...@gmail.com writes:
 On Tue, May 6, 2014 at 2:34 PM, Tom Lane t...@sss.pgh.pa.us wrote:
 FWIW, I vote for fixing (a) now but holding (b) for 9.5.

 I guess I'll vote for applying both.  I don't see a lot of risk, and I
 think doing one with out the other is somewhat pointless.

 The difference is that there's not consensus about the details of the
 views ... as borne out by your next paragraph.

 Now admittedly, we could always redefine the views in 9.5, but
 I'd rather not be doing this sort of thing in haste.  Something
 as user-visible as a system view really ought to have baked awhile
 before we ship it.  Patch (a) is merely institutionalizing the
 expectation that DSM segments should have names, which is a much
 lower-risk bet.

As long as all the functions are exposed to allow b) to run as an
extension, I don't see we lose anything by waiting a while.

-- 
 Simon Riggs   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

Re: [HACKERS] pg_shmem_allocations view

[Andres Freund]

On 2014-05-06 15:37:04 -0400, Robert Haas wrote:
 On Tue, May 6, 2014 at 2:34 PM, Tom Lane t...@sss.pgh.pa.us wrote:
  Andres Freund and...@2ndquadrant.com writes:
  Attached are two patches:
  a) Patch addin a 'name' parameter to dsm_create(). I think we should
 apply this to 9.4.
  b) pg_dynamic_shmem_allocations and pg_static_shmem_allocations
 views. The previous version didn't include dsm support and didn't
 take the required lock.
 
  I am not so sure whether b) should be applied together with a) in 9.4,
  but I'd be happy enough to add docs if people agree with the naming.
 
  FWIW, I vote for fixing (a) now but holding (b) for 9.5.
 
 I guess I'll vote for applying both.  I don't see a lot of risk, and I
 think doing one with out the other is somewhat pointless.

Fine with me. I guess if we don't do b) for now we can just do the
additional parameter and the Assert() from the patch. Without actually
storing the name to shared memory.

 Regarding patch 0002, I don't think we're using the term static
 shmem anywhere else, so I vote for dropping the word static, so that
 we have pg_get_shmem_allocations() and
 pg_get_dynamic_shmem_allocations().

Fine #2.

  Also, I think using the
 allocated column is pretty weird.  There are always exactly two
 entries with allocated = false

Hm. There shouldn't be. And at least in my installation there isn't and
I don't see a anything in the code that'd allow that? The example from
my last email has:

 postgres=# SELECT * FROM pg_static_shmem_allocations ORDER BY key NULLS FIRST;
  key |off |size| allocated
 -+++---
  | 605024 |1727776 | f
  ||   34844752 | t
  Async Ctl   | 539168 |  65856 | t

, one of which is for actual free memory
 and the other of which is for memory that actually IS allocated but
 without using ShmemIndex (maybe the latter was supposed to have
 allocated = true but still key = null?).

Yes, that's how I'd imagined it.

 I guess I'd vote for
 ditching the allocated column completely and outputting the memory
 allocated without ShmemIndex using some fixed tag (like ShmemIndex
 or Bootstrap or Overhead or something).

My way feels slightly cleaner, but I'd be ok with that as well. There's
no possible conflicts with an actual segment... In your variant the
unallocated/slop memory would continue to have a NULL key?

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

Re: [HACKERS] pg_shmem_allocations view

[Andres Freund]

On 2014-05-06 22:04:04 +0100, Simon Riggs wrote:
 On 6 May 2014 20:44, Tom Lane t...@sss.pgh.pa.us wrote:
  Robert Haas robertmh...@gmail.com writes:
  On Tue, May 6, 2014 at 2:34 PM, Tom Lane t...@sss.pgh.pa.us wrote:
  FWIW, I vote for fixing (a) now but holding (b) for 9.5.
 
  I guess I'll vote for applying both.  I don't see a lot of risk, and I
  think doing one with out the other is somewhat pointless.
 
  The difference is that there's not consensus about the details of the
  views ... as borne out by your next paragraph.
 
  Now admittedly, we could always redefine the views in 9.5, but
  I'd rather not be doing this sort of thing in haste.  Something
  as user-visible as a system view really ought to have baked awhile
  before we ship it.  Patch (a) is merely institutionalizing the
  expectation that DSM segments should have names, which is a much
  lower-risk bet.
 
 As long as all the functions are exposed to allow b) to run as an
 extension, I don't see we lose anything by waiting a while.

They aren't exposed. It's touching implementation details in both
shmem.c and dsm.c. I think that's actually fine.
Imo it's not too bad if we don't get either in 9.4. It's not a critical
feature.What I *would* like to avoid is a pointless API break between
9.4 and 9.5. Because I will push for the patch in 9.5 CF1...

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

Re: [HACKERS] pg_shmem_allocations view

[Robert Haas]

On Sun, May 4, 2014 at 7:50 AM, Andres Freund and...@2ndquadrant.com wrote:
 On 2014-05-04 13:44:17 +0200, Andres Freund wrote:
 postgres=# SELECT * FROM pg_shmem_allocations ORDER BY size DESC;
  key | off |size | allocated
 -+-+-+---
  Buffer Blocks   |   286242528 | 17179869184 | t
  Buffer Descriptors  |   152024800 |   134217728 | t
 ...
  OldSerXidControlData| 17584357344 |  16 | t
 (44 rows)

 Thinking about this, I think it was a mistake to not add a 'name' field
 to dynamic shared memory's dsm_control_item. Right now it's very hard to
 figure out which extension allocated a dsm segment. Imo we should change
 that before 9.4 is out. I am not suggesting to use it to identify
 segments, but just as an identifier, passed in into dsm_create().

 Imo there should be a corresponding pg_dynshmem_allocations to
 pg_shmem_allocations.

Well, right now a dsm_control_item is 8 bytes.  If we add a name field
of our usual 64 bytes, they'll each be 9 times bigger.  We're not
talking about a lot of bytes in absolute terms, but I guess I'm not in
favor of an 800% size increase without somewhat more justification
than you've provided here.  Who is using dynamic shared memory for
enough different things at this point to get confused?

I'm quite in favor of having something like this for the main shared
memory segment, but I think that's 9.5 material at this point.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


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

Re: [HACKERS] pg_shmem_allocations view

[Tom Lane]

Robert Haas robertmh...@gmail.com writes:
 On Sun, May 4, 2014 at 7:50 AM, Andres Freund and...@2ndquadrant.com wrote:
 Thinking about this, I think it was a mistake to not add a 'name' field
 to dynamic shared memory's dsm_control_item.

 Well, right now a dsm_control_item is 8 bytes.  If we add a name field
 of our usual 64 bytes, they'll each be 9 times bigger.

And the controlled shared segment is likely to be how big exactly?  It's
probably not even possible for it to be smaller than a page size, 4K or
so depending on the OS.  I agree with Andres that a name would be a good
idea; complaining about the space needed to hold it is penny-wise and
pound-foolish.

 I'm quite in favor of having something like this for the main shared
 memory segment, but I think that's 9.5 material at this point.

If you're prepared to break the current APIs later to add a name parameter
(which would have to be required, if it's to be useful at all), then sure,
put the question off till 9.5.

regards, tom lane


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

Re: [HACKERS] pg_shmem_allocations view

[Andres Freund]

On 2014-05-05 15:04:07 -0400, Robert Haas wrote:
 On Sun, May 4, 2014 at 7:50 AM, Andres Freund and...@2ndquadrant.com wrote:
  On 2014-05-04 13:44:17 +0200, Andres Freund wrote:
  postgres=# SELECT * FROM pg_shmem_allocations ORDER BY size DESC;
   key | off |size | 
  allocated
  -+-+-+---
   Buffer Blocks   |   286242528 | 17179869184 | t
   Buffer Descriptors  |   152024800 |   134217728 | t
  ...
   OldSerXidControlData| 17584357344 |  16 | t
  (44 rows)
 
  Thinking about this, I think it was a mistake to not add a 'name' field
  to dynamic shared memory's dsm_control_item. Right now it's very hard to
  figure out which extension allocated a dsm segment. Imo we should change
  that before 9.4 is out. I am not suggesting to use it to identify
  segments, but just as an identifier, passed in into dsm_create().
 
  Imo there should be a corresponding pg_dynshmem_allocations to
  pg_shmem_allocations.
 
 Well, right now a dsm_control_item is 8 bytes.  If we add a name field
 of our usual 64 bytes, they'll each be 9 times bigger.  We're not
 talking about a lot of bytes in absolute terms, but I guess I'm not in
 favor of an 800% size increase without somewhat more justification
 than you've provided here.  Who is using dynamic shared memory for
 enough different things at this point to get confused?

The kernel side overhead of creating a shared memory segment are so much
higher that this really isn't a meaningful saving. Also, are you really
considering a couple hundred bytes to be a problem?
I think it's quite a sensible thing for an administrator to ask where
all the memory has gone. The more users for dsm there the more important
that'll get. Right now pretty much the only thing a admin could do is to
poke around in /proc to see which backend has mapped the segment and try
to figure out via the logs what caused it to do so. Not nice.

 I'm quite in favor of having something like this for the main shared
 memory segment, but I think that's 9.5 material at this point.

Clearly. For one the version I posted here missed allocations which
aren't done via ShmemInitStruct (lwlock main array and hash table
allocations primarily). For another it's too late ;)

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

Re: [HACKERS] pg_shmem_allocations view

[Andres Freund]

On 2014-05-05 15:09:02 -0400, Tom Lane wrote:
  I'm quite in favor of having something like this for the main shared
  memory segment, but I think that's 9.5 material at this point.
 
 If you're prepared to break the current APIs later to add a name parameter
 (which would have to be required, if it's to be useful at all), then sure,
 put the question off till 9.5.

I understood Robert to mean that it's too late for my proposed view for
9.4 - and I agree - but I wholeheartedly agree with you that we should
add a name parameter to the dsm API *now*. We can just Assert() that it's
nonzero if we don't think it's useful for now.

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

Re: [HACKERS] pg_shmem_allocations view

[Robert Haas]

On Mon, May 5, 2014 at 3:09 PM, Tom Lane t...@sss.pgh.pa.us wrote:
 Robert Haas robertmh...@gmail.com writes:
 On Sun, May 4, 2014 at 7:50 AM, Andres Freund and...@2ndquadrant.com wrote:
 Thinking about this, I think it was a mistake to not add a 'name' field
 to dynamic shared memory's dsm_control_item.

 Well, right now a dsm_control_item is 8 bytes.  If we add a name field
 of our usual 64 bytes, they'll each be 9 times bigger.

 And the controlled shared segment is likely to be how big exactly?  It's
 probably not even possible for it to be smaller than a page size, 4K or
 so depending on the OS.  I agree with Andres that a name would be a good
 idea; complaining about the space needed to hold it is penny-wise and
 pound-foolish.

The control segment is sized to support a number of dynamic shared
memory segments not exceeding 64 + 2 *MaxBackends.  With default
settings, that currently works out to 288 segments, or 2306 bytes.
So, adding a 64-byte name to each of those structures would increase
the size from 2k to about 20k.

So, sure, that's not a lot of memory.  But I'm still not convinced
that's it's very useful.  What I think is going to happen is that (1)
most people won't be used dynamic shared memory at all, so they won't
have any use for this; (2) those people who do run an extension that
uses dynamic shared memory will most likely only be running one such
extension, so they won't need a name to know what the segments are
being used for; and (3) if and when we eventually get parallel query,
dynamic shared memory segments will be widely used, but a bunch of
segments that are all named parallel_query or parallel_query.$PID
isn't going to be too informative.

Now, all that having been said, I recognize that human-readable names
are a generally useful thing, so I'm not going to hold my breath until
I turn blue if other people really want this, and it may turn out to
be useful someday.  But if anyone is curious whether I'm *confident*
that it will be useful someday: at this point, no.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


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

Re: [HACKERS] pg_shmem_allocations view

[Tom Lane]

Robert Haas robertmh...@gmail.com writes:
 On Mon, May 5, 2014 at 3:09 PM, Tom Lane t...@sss.pgh.pa.us wrote:
 And the controlled shared segment is likely to be how big exactly?  It's
 probably not even possible for it to be smaller than a page size, 4K or
 so depending on the OS.  I agree with Andres that a name would be a good
 idea; complaining about the space needed to hold it is penny-wise and
 pound-foolish.
 ...
 Now, all that having been said, I recognize that human-readable names
 are a generally useful thing, so I'm not going to hold my breath until
 I turn blue if other people really want this, and it may turn out to
 be useful someday.  But if anyone is curious whether I'm *confident*
 that it will be useful someday: at this point, no.

I'm not confident that it'll be useful either.  But I am confident that
if we don't put it in now, and decide we want it later, there will be
complaints when we change the API.  Better to have an ignored parameter
than no parameter.

regards, tom lane


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

Re: [HACKERS] pg_shmem_allocations view

[Robert Haas]

On Mon, May 5, 2014 at 6:54 PM, Tom Lane t...@sss.pgh.pa.us wrote:
 Robert Haas robertmh...@gmail.com writes:
 On Mon, May 5, 2014 at 3:09 PM, Tom Lane t...@sss.pgh.pa.us wrote:
 And the controlled shared segment is likely to be how big exactly?  It's
 probably not even possible for it to be smaller than a page size, 4K or
 so depending on the OS.  I agree with Andres that a name would be a good
 idea; complaining about the space needed to hold it is penny-wise and
 pound-foolish.
 ...
 Now, all that having been said, I recognize that human-readable names
 are a generally useful thing, so I'm not going to hold my breath until
 I turn blue if other people really want this, and it may turn out to
 be useful someday.  But if anyone is curious whether I'm *confident*
 that it will be useful someday: at this point, no.

 I'm not confident that it'll be useful either.  But I am confident that
 if we don't put it in now, and decide we want it later, there will be
 complaints when we change the API.  Better to have an ignored parameter
 than no parameter.

I'm generally skeptical of that philosophy.  If we put in an ignored
parameter, people may pass pointers to NULL or to garbage or to an
overly-long string, and they won't know it's broken until we make it
do something; at which point their code will begin to fail without
warning.  Speaking as an employee of a company that maintains several
PostgreSQL extensions that sometimes need to be updated for newer
server versions, I'd rather have a clean API break that makes the
build fail than a soft break that supposedly lets things continue
working but maybe breaks them in subtler ways.  Another problem with
this idea is that we might never get around to making it do anything,
and then the dead parameter is just a stupid and unnecessary wart.

If we're going to do anything at all here for 9.4, I recommend
ignoring the fact we're in feature freeze and going whole hog: add the
name, add the monitoring view, and add the monitoring view for the
main shared memory segment just for good measure.  That way, if we get
the design wrong or something, we have a chance of getting some
feedback.  If we're not going to do that, then I vote for doing
nothing and considering later whether to break it for 9.5, by which
time we may have some evidence as to whether and how this code is
really being used.  Anyone who expects PostgreSQL's C API to be
completely stable is going to be regularly disappointed, as most
recently demonstrated by the Enormous Header Churn of the 9.3 era.  I
don't particularly mind being the cause of further disappointment; as
long as the breakage is obvious rather than subtle, the fix usually
takes about 10 minutes.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


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

Re: [HACKERS] pg_shmem_allocations view

[Andres Freund]

Hi,

On 2014-05-04 13:44:17 +0200, Andres Freund wrote:
 postgres=# SELECT * FROM pg_shmem_allocations ORDER BY size DESC;
  key | off |size | allocated
 -+-+-+---
  Buffer Blocks   |   286242528 | 17179869184 | t
  Buffer Descriptors  |   152024800 |   134217728 | t
 ...
  OldSerXidControlData| 17584357344 |  16 | t
 (44 rows)

Thinking about this, I think it was a mistake to not add a 'name' field
to dynamic shared memory's dsm_control_item. Right now it's very hard to
figure out which extension allocated a dsm segment. Imo we should change
that before 9.4 is out. I am not suggesting to use it to identify
segments, but just as an identifier, passed in into dsm_create().

Imo there should be a corresponding pg_dynshmem_allocations to
pg_shmem_allocations.

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

Re: [HACKERS] pg_shmem_allocations view

[Andres Freund]

Hi,

On 2014-05-04 13:44:17 +0200, Andres Freund wrote:
 postgres=# SELECT * FROM pg_shmem_allocations ORDER BY size DESC;
  key | off |size | allocated
 -+-+-+---
  Buffer Blocks   |   286242528 | 17179869184 | t
  Buffer Descriptors  |   152024800 |   134217728 | t

Abhijit notified me that I've attached the wrong patch. Corrected.

Greetings,

Andres Freund

-- 
 Andres Freund http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training  Services
From e8a7576f3a593f4f88bd619ae2504ee320e61db2 Mon Sep 17 00:00:00 2001
From: Andres Freund and...@anarazel.de
Date: Sun, 4 May 2014 13:37:20 +0200
Subject: [PATCH] Add pg_shmem_allocations view.

---
 src/backend/catalog/system_views.sql |  3 ++
 src/backend/storage/ipc/shmem.c  | 97 
 src/include/catalog/pg_proc.h|  2 +
 src/include/utils/builtins.h |  3 ++
 src/test/regress/expected/rules.out  |  5 ++
 5 files changed, 110 insertions(+)

diff --git a/src/backend/catalog/system_views.sql b/src/backend/catalog/system_views.sql
index 42a4c00..104491a 100644
--- a/src/backend/catalog/system_views.sql
+++ b/src/backend/catalog/system_views.sql
@@ -387,6 +387,9 @@ CREATE VIEW pg_timezone_abbrevs AS
 CREATE VIEW pg_timezone_names AS
 SELECT * FROM pg_timezone_names();
 
+CREATE VIEW pg_shmem_allocations AS
+SELECT * FROM pg_get_shmem_allocations();
+
 -- Statistics views
 
 CREATE VIEW pg_stat_all_tables AS
diff --git a/src/backend/storage/ipc/shmem.c b/src/backend/storage/ipc/shmem.c
index 1d27a89..5722c78 100644
--- a/src/backend/storage/ipc/shmem.c
+++ b/src/backend/storage/ipc/shmem.c
@@ -66,11 +66,14 @@
 #include postgres.h
 
 #include access/transam.h
+#include fmgr.h
+#include funcapi.h
 #include miscadmin.h
 #include storage/lwlock.h
 #include storage/pg_shmem.h
 #include storage/shmem.h
 #include storage/spin.h
+#include utils/builtins.h
 
 
 /* shared memory global variables */
@@ -459,3 +462,97 @@ mul_size(Size s1, Size s2)
  errmsg(requested shared memory size overflows size_t)));
 	return result;
 }
+
+/* SQL SRF showing allocated shared memory */
+Datum
+pg_get_shmem_allocations(PG_FUNCTION_ARGS)
+{
+#define PG_GET_SHMEM_SIZES_COLS 4
+
+	ReturnSetInfo *rsinfo = (ReturnSetInfo *) fcinfo-resultinfo;
+	TupleDesc	tupdesc;
+	Tuplestorestate *tupstore;
+	MemoryContext per_query_ctx;
+	MemoryContext oldcontext;
+	HASH_SEQ_STATUS hstat;
+	ShmemIndexEnt *ent;
+
+	/* check to see if caller supports us returning a tuplestore */
+	if (rsinfo == NULL || !IsA(rsinfo, ReturnSetInfo))
+		ereport(ERROR,
+(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+ errmsg(set-valued function called in context that cannot accept a set)));
+	if (!(rsinfo-allowedModes  SFRM_Materialize))
+		ereport(ERROR,
+(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+ errmsg(materialize mode required, but it is not  \
+		allowed in this context)));
+
+	/* Build a tuple descriptor for our result type */
+	if (get_call_result_type(fcinfo, NULL, tupdesc) != TYPEFUNC_COMPOSITE)
+		elog(ERROR, return type must be a row type);
+
+	per_query_ctx = rsinfo-econtext-ecxt_per_query_memory;
+	oldcontext = MemoryContextSwitchTo(per_query_ctx);
+
+	tupstore = tuplestore_begin_heap(true, false, work_mem);
+	rsinfo-returnMode = SFRM_Materialize;
+	rsinfo-setResult = tupstore;
+	rsinfo-setDesc = tupdesc;
+
+	MemoryContextSwitchTo(oldcontext);
+
+	hash_seq_init(hstat, ShmemIndex);
+
+	/* output all allocated entries */
+	while ((ent = (ShmemIndexEnt *) hash_seq_search(hstat)) != NULL)
+	{
+		Datum		values[PG_GET_SHMEM_SIZES_COLS];
+		bool		nulls[PG_GET_SHMEM_SIZES_COLS];
+
+		/* key */
+		values[0] = CStringGetTextDatum(ent-key);
+		nulls[0] = false;
+
+		/* off */
+		values[1] = Int64GetDatum((char *) ent-location - (char *) ShmemSegHdr);
+		nulls[1] = false;
+
+		/* size */
+		values[2] = Int64GetDatum(ent-size);
+		nulls[2] = false;
+
+		/* allocated */
+		values[3] = BoolGetDatum(true);
+		nulls[3] = false;
+
+		tuplestore_putvalues(tupstore, tupdesc, values, nulls);
+	}
+
+	/* output as-of-yet unallocated memory */
+	{
+		Datum		values[PG_GET_SHMEM_SIZES_COLS];
+		bool		nulls[PG_GET_SHMEM_SIZES_COLS];
+
+		/* key, show unallocated as NULL */
+		nulls[0] = true;
+
+		/* off */
+		values[1] = Int64GetDatum(ShmemSegHdr-freeoffset);
+		nulls[1] = false;
+
+		/* size */
+		values[2] = Int64GetDatum(ShmemSegHdr-totalsize - ShmemSegHdr-freeoffset);
+		nulls[2] = false;
+
+		/* allocated */
+		values[3] = BoolGetDatum(false);
+		nulls[3] = false;
+
+		tuplestore_putvalues(tupstore, tupdesc, values, nulls);
+	}
+
+	tuplestore_donestoring(tupstore);
+
+	return (Datum) 0;
+}
diff --git a/src/include/catalog/pg_proc.h b/src/include/catalog/pg_proc.h
index 98c183b..d018e6f 100644
--- a/src/include/catalog/pg_proc.h
+++ b/src/include/catalog/pg_proc.h
@@ -3899,6 

Re: [HACKERS] pg_shmem_allocations view

[Euler Taveira]

On 04-05-2014 08:44, Andres Freund wrote:
 I've more than once wanted to know what allocated shared memory in
 postgres installation is used for. Especially with more an more
 extensions around that's quite useful.
 
A few years ago I had to provide such information an did something
similar. Is it useful? Yes. However, it is a developer's feature.

 On 2014-05-04 13:44:17 +0200, Andres Freund wrote:
 Thinking about this, I think it was a mistake to not add a 'name' field
 to dynamic shared memory's dsm_control_item. Right now it's very hard to
 figure out which extension allocated a dsm segment. Imo we should change
 that before 9.4 is out. I am not suggesting to use it to identify
 segments, but just as an identifier, passed in into dsm_create().
 
+1.

 Imo there should be a corresponding pg_dynshmem_allocations to
 pg_shmem_allocations.

... or another boolean column (say 'dynamic') and just one view.


-- 
   Euler Taveira   Timbira - http://www.timbira.com.br/
   PostgreSQL: Consultoria, Desenvolvimento, Suporte 24x7 e Treinamento


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