Retain dynamic shared memory segments for postmaster lifetime
Currently there is no way user can keep the dsm
segments if he wants for postmaster lifetime, so I
have exposed a new API dsm_keep_segment()
to implement the same.
The specs and need for this API is already discussed
in thread:
/messages/by-id/CA+TgmoaKoGuJQbEdGeYKYSXud9EAidqx77J2_HXzRgFo3Hr46A@mail.gmail.com
I had used dsm_demo (hacked it a bit) module used
during initial tests for dsm API's to verify the working on
Windows. So one idea could be that I can extend
that module to use this new API, so that it can be tested
by others as well or if you have any other better way, please
do let me know.
As the discussion about its specs and need is already
discussed in above mentioned thread, so I will upload
this patch to CF unless there is any Objection.
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com
Attachments:
dsm_keep_segment_v1.patchapplication/octet-stream; name=dsm_keep_segment_v1.patchDownload
diff --git a/src/backend/storage/ipc/dsm.c b/src/backend/storage/ipc/dsm.c
index 4ee453e..11c913a 100644
--- a/src/backend/storage/ipc/dsm.c
+++ b/src/backend/storage/ipc/dsm.c
@@ -42,6 +42,7 @@
#include "utils/guc.h"
#include "utils/memutils.h"
#include "utils/resowner_private.h"
+#include "postmaster/postmaster.h"
#define PG_DYNSHMEM_STATE_FILE PG_DYNSHMEM_DIR "/state"
#define PG_DYNSHMEM_NEW_STATE_FILE PG_DYNSHMEM_DIR "/state.new"
@@ -886,6 +887,52 @@ dsm_keep_mapping(dsm_segment *seg)
}
/*
+ * Keep a dynamic shared memory segment until end of postmaster.
+ *
+ * By default, segments are owned by the current resource owner, which
+ * typically means they stick around for the duration of the current query
+ * only.
+ */
+void
+dsm_keep_segment(dsm_segment *seg)
+{
+ /*
+ * Bump reference count for this segment in shared memory. This will
+ * ensure that even if there is no session which is attached to this
+ * segment, it will stay till postmaster lifetime. For windows, we
+ * need to duplicate segment handle for postmaster process, as it
+ * automatically drops segment if all referring sessions end.
+ */
+ LWLockAcquire(DynamicSharedMemoryControlLock, LW_EXCLUSIVE);
+ dsm_control->item[seg->control_slot].refcnt++;
+ LWLockRelease(DynamicSharedMemoryControlLock);
+
+#ifdef WIN32
+ {
+ HANDLE hmap2;
+
+ if (!DuplicateHandle(GetCurrentProcess(),
+ seg->impl_private,
+ PostmasterHandle,
+ &hmap2,
+ 0,
+ FALSE,
+ DUPLICATE_SAME_ACCESS))
+ {
+ char name[64];
+
+ snprintf(name, 64, "Global/PostgreSQL.%u", seg->handle);
+ _dosmaperr(GetLastError());
+ ereport(ERROR,
+ (errcode_for_file_access(),
+ errmsg("could not retain segment \"%s\" for postmaster lifetime: %m",
+ name)));
+ }
+ }
+#endif
+}
+
+/*
* Find an existing mapping for a shared memory segment, if there is one.
*/
dsm_segment *
diff --git a/src/include/storage/dsm.h b/src/include/storage/dsm.h
index 71901bf..16afef3 100644
--- a/src/include/storage/dsm.h
+++ b/src/include/storage/dsm.h
@@ -30,6 +30,7 @@ extern void dsm_detach(dsm_segment *seg);
/* Resource management functions. */
extern void dsm_keep_mapping(dsm_segment *seg);
+extern void dsm_keep_segment(dsm_segment *seg);
extern dsm_segment *dsm_find_mapping(dsm_handle h);
/* Informational functions. */
On Sun, Jan 12, 2014 at 12:41 PM, Amit Kapila <amit.kapila16@gmail.com> wrote:
Currently there is no way user can keep the dsm
segments if he wants for postmaster lifetime, so I
have exposed a new API dsm_keep_segment()
to implement the same.The specs and need for this API is already discussed
in thread:
/messages/by-id/CA+TgmoaKoGuJQbEdGeYKYSXud9EAidqx77J2_HXzRgFo3Hr46A@mail.gmail.com
We have decided to bump reference count for segment
and call DuplicateHandle for Windows, but I think it should
also do what dsm_keep_mapping() does that is
ResourceOwnerForgetDSM(), else it will give
Warning: dynamic shared memory leak at transaction end.
I had used dsm_demo (hacked it a bit) module used
during initial tests for dsm API's to verify the working on
Windows. So one idea could be that I can extend
that module to use this new API, so that it can be tested
by others as well or if you have any other better way, please
do let me know.
I have extended test (contrib) module dsm_demo such that now user
can specify during dsm_demo_create the lifespan of segment.
The values it can accept are 0 or 1. Default value is 0.
0 -- means segment will be accessible for session life time
1 -- means segment will be accessible for postmaster life time
The behaviour is as below:
Test -1 (Session life time)
Session - 1
-- here it will create segment for session lifetime
select dsm_demo_create('this message is from session-1', 0);
dsm_demo_create
-----------------
827121111
Session - 2
-----------------
select dsm_demo_read(827121111);
dsm_demo_read
----------------------------
this message is from session-1
(1 row)
Session-1
\q
Session-2
postgres=# select dsm_demo_read(827121111);
dsm_demo_read
---------------
(1 row)
Conclusion of Test-1 : As soon as session which has created segment finished,
the segment becomes non-accessible.
Test -2 (Postmaster life time)
Session - 1
-- here it will create segment for postmaster lifetime
select dsm_demo_create('this message is from session-1', 1);
dsm_demo_create
-----------------
827121111
Session - 2
-----------------
select dsm_demo_read(827121111);
dsm_demo_read
----------------------------
this message is from session-1
(1 row)
Session-1
\q
Session-2
postgres=# select dsm_demo_read(827121111);
dsm_demo_read
---------------
this message is from session-1
(1 row)
Conclusion of Test-2 : a. Segment is accessible for postmaster lifetime.
b. if user restart server, segment is
not accessible.
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com
Attachments:
dsm_demo_v1.patchapplication/octet-stream; name=dsm_demo_v1.patchDownload
diff --git a/contrib/Makefile b/contrib/Makefile
index dd2683b..c4284d9 100644
--- a/contrib/Makefile
+++ b/contrib/Makefile
@@ -16,6 +16,7 @@ SUBDIRS = \
dblink \
dict_int \
dict_xsyn \
+ dsm_demo \
dummy_seclabel \
earthdistance \
file_fdw \
diff --git a/contrib/dsm_demo/Makefile b/contrib/dsm_demo/Makefile
new file mode 100644
index 0000000..dd9ea92
--- /dev/null
+++ b/contrib/dsm_demo/Makefile
@@ -0,0 +1,17 @@
+# contrib/dsm_demo/Makefile
+
+MODULES = dsm_demo
+
+EXTENSION = dsm_demo
+DATA = dsm_demo--1.0.sql
+
+ifdef USE_PGXS
+PG_CONFIG = pg_config
+PGXS := $(shell $(PG_CONFIG) --pgxs)
+include $(PGXS)
+else
+subdir = contrib/dsm_demo
+top_builddir = ../..
+include $(top_builddir)/src/Makefile.global
+include $(top_srcdir)/contrib/contrib-global.mk
+endif
diff --git a/contrib/dsm_demo/dsm_demo--1.0.sql b/contrib/dsm_demo/dsm_demo--1.0.sql
new file mode 100644
index 0000000..77d5190
--- /dev/null
+++ b/contrib/dsm_demo/dsm_demo--1.0.sql
@@ -0,0 +1,14 @@
+/* contrib/dsm_demo/dsm_demo--1.0.sql */
+
+-- complain if script is sourced in psql, rather than via CREATE EXTENSION
+\echo Use "CREATE EXTENSION dsm_demo" to load this file. \quit
+
+CREATE FUNCTION dsm_demo_create(pg_catalog.text, pg_catalog.int4 default 0)
+RETURNS pg_catalog.int8 STRICT
+AS 'MODULE_PATHNAME'
+LANGUAGE C;
+
+CREATE FUNCTION dsm_demo_read(pg_catalog.int8)
+RETURNS pg_catalog.text STRICT
+AS 'MODULE_PATHNAME'
+LANGUAGE C;
\ No newline at end of file
diff --git a/contrib/dsm_demo/dsm_demo.c b/contrib/dsm_demo/dsm_demo.c
new file mode 100644
index 0000000..7beb455
--- /dev/null
+++ b/contrib/dsm_demo/dsm_demo.c
@@ -0,0 +1,113 @@
+/* -------------------------------------------------------------------------
+ *
+ * dsm_demo.c
+ * Dynamic shared memory demonstration.
+ *
+ * Copyright (C) 2013, PostgreSQL Global Development Group
+ *
+ * IDENTIFICATION
+ * contrib/dsm_demo/dsm_demo.c
+ *
+ * -------------------------------------------------------------------------
+ */
+
+#include "postgres.h"
+
+#include "storage/dsm.h"
+#include "fmgr.h"
+
+PG_MODULE_MAGIC;
+
+void _PG_init(void);
+Datum dsm_demo_create(PG_FUNCTION_ARGS);
+Datum dsm_demo_read(PG_FUNCTION_ARGS);
+
+PG_FUNCTION_INFO_V1(dsm_demo_create);
+PG_FUNCTION_INFO_V1(dsm_demo_read);
+
+#define DSM_DEMO_MAGIC 0x44454D4F
+
+typedef struct
+{
+ uint32 magic;
+ int32 len;
+ char data[FLEXIBLE_ARRAY_MEMBER];
+} dsm_demo_payload;
+
+/*
+ * dsm_demo_create(text, segment_lifetime int4)
+ *
+ * The first argument is the text data to be shared by using dsm, the second
+ * controls the life span of dsm segemnt. 0 indicates session lifetime, 1
+ * indicates postmaster lifetime.
+ */
+Datum
+dsm_demo_create(PG_FUNCTION_ARGS)
+{
+ text *txt = PG_GETARG_TEXT_PP(0);
+ int len = VARSIZE_ANY(txt);
+ int segment_life = PG_GETARG_INT32(1);
+ uint64 seglen;
+ dsm_segment *seg;
+ dsm_demo_payload *payload;
+
+ seglen = offsetof(dsm_demo_payload, data) + len;
+ seg = dsm_create(seglen);
+
+ if (segment_life == 0)
+ dsm_keep_mapping(seg);
+ else if (segment_life == 1)
+ dsm_keep_segment(seg);
+ else
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ (errmsg("value of segment lifetime can be either 0 or 1"))));
+
+ payload = dsm_segment_address(seg);
+ payload->magic = DSM_DEMO_MAGIC;
+ payload->len = len;
+ memcpy(payload->data, txt, len);
+
+ PG_RETURN_INT64(dsm_segment_handle(seg));
+}
+
+Datum
+dsm_demo_read(PG_FUNCTION_ARGS)
+{
+ dsm_handle h = PG_GETARG_INT64(0);
+ dsm_segment *seg;
+ bool needs_detach = false;
+ text *txt = NULL;
+ dsm_demo_payload *payload;
+
+ /*
+ * We could be called from the same sesion that called dsm_demo_create(),
+ * so search for an existing mapping. If we don't find one, attach the
+ * segment.
+ */
+ seg = dsm_find_mapping(h);
+ if (seg == NULL)
+ {
+ seg = dsm_attach(h);
+ if (!seg)
+ PG_RETURN_NULL();
+ needs_detach = true;
+ }
+
+ /* Extract data, after checking magic number. */
+ payload = dsm_segment_address(seg);
+ if (payload->magic == DSM_DEMO_MAGIC)
+ {
+ txt = palloc(payload->len);
+ memcpy(txt, payload->data, payload->len);
+ }
+
+ /* Detach, if there was no existing mapping. */
+ if (needs_detach)
+ dsm_detach(seg);
+
+ if (txt == NULL)
+ PG_RETURN_NULL();
+
+ PG_RETURN_TEXT_P(txt);
+}
diff --git a/contrib/dsm_demo/dsm_demo.control b/contrib/dsm_demo/dsm_demo.control
new file mode 100644
index 0000000..4060791
--- /dev/null
+++ b/contrib/dsm_demo/dsm_demo.control
@@ -0,0 +1,5 @@
+# dsm_demo extension
+comment = 'Dynamic shared memory demonstration'
+default_version = '1.0'
+module_pathname = 'dsm_demo'
+relocatable = true
On Mon, Jan 13, 2014 at 2:50 PM, Amit Kapila <amit.kapila16@gmail.com> wrote:
I have extended test (contrib) module dsm_demo such that now user
can specify during dsm_demo_create the lifespan of segment.
The values it can accept are 0 or 1. Default value is 0.
0 -- means segment will be accessible for session life time
1 -- means segment will be accessible for postmaster life timeThe behaviour is as below:
Test -1 (Session life time)
Session - 1
-- here it will create segment for session lifetime
select dsm_demo_create('this message is from session-1', 0);
dsm_demo_create
-----------------
827121111Session - 2
-----------------
select dsm_demo_read(827121111);
dsm_demo_read
----------------------------
this message is from session-1
(1 row)Session-1
\qSession-2
postgres=# select dsm_demo_read(827121111);
dsm_demo_read
---------------(1 row)
Conclusion of Test-1 : As soon as session which has created segment finished,
the segment becomes non-accessible.Test -2 (Postmaster life time)
Session - 1
-- here it will create segment for postmaster lifetime
select dsm_demo_create('this message is from session-1', 1);
dsm_demo_create
-----------------
827121111Session - 2
-----------------
select dsm_demo_read(827121111);
dsm_demo_read
----------------------------
this message is from session-1
(1 row)Session-1
\qSession-2
postgres=# select dsm_demo_read(827121111);
dsm_demo_read
---------------
this message is from session-1
(1 row)Conclusion of Test-2 : a. Segment is accessible for postmaster lifetime.
b. if user restart server, segment is
not accessible.
Applied dsm_keep_segment_v1.patch and dsm_demo_v1.patch.
Got the following warning when I tried above example:
postgres=# select dsm_demo_create('this message is from session-new', 1);
WARNING: dynamic shared memory leak: segment 1402373971 still referenced
WARNING: dynamic shared memory leak: segment 1402373971 still referenced
dsm_demo_create
-----------------
1402373971
(1 row)
--
Amit
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Mon, Jan 27, 2014 at 11:18 PM, Amit Langote <amitlangote09@gmail.com> wrote:
On Mon, Jan 13, 2014 at 2:50 PM, Amit Kapila <amit.kapila16@gmail.com> wrote:
I have extended test (contrib) module dsm_demo such that now user
can specify during dsm_demo_create the lifespan of segment.
The values it can accept are 0 or 1. Default value is 0.
0 -- means segment will be accessible for session life time
1 -- means segment will be accessible for postmaster life timeThe behaviour is as below:
Test -1 (Session life time)
Session - 1
-- here it will create segment for session lifetime
select dsm_demo_create('this message is from session-1', 0);
dsm_demo_create
-----------------
827121111Session - 2
-----------------
select dsm_demo_read(827121111);
dsm_demo_read
----------------------------
this message is from session-1
(1 row)Session-1
\qSession-2
postgres=# select dsm_demo_read(827121111);
dsm_demo_read
---------------(1 row)
Conclusion of Test-1 : As soon as session which has created segment finished,
the segment becomes non-accessible.Test -2 (Postmaster life time)
Session - 1
-- here it will create segment for postmaster lifetime
select dsm_demo_create('this message is from session-1', 1);
dsm_demo_create
-----------------
827121111Session - 2
-----------------
select dsm_demo_read(827121111);
dsm_demo_read
----------------------------
this message is from session-1
(1 row)Session-1
\qSession-2
postgres=# select dsm_demo_read(827121111);
dsm_demo_read
---------------
this message is from session-1
(1 row)Conclusion of Test-2 : a. Segment is accessible for postmaster lifetime.
b. if user restart server, segment is
not accessible.Applied dsm_keep_segment_v1.patch and dsm_demo_v1.patch.
Got the following warning when I tried above example:postgres=# select dsm_demo_create('this message is from session-new', 1);
WARNING: dynamic shared memory leak: segment 1402373971 still referenced
WARNING: dynamic shared memory leak: segment 1402373971 still referenced
dsm_demo_create
-----------------
1402373971
(1 row)
I see that PrintDSMLeakWarning() which emits this warning is for debugging.
--
Amit
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Mon, Jan 27, 2014 at 7:48 PM, Amit Langote <amitlangote09@gmail.com> wrote:
On Mon, Jan 13, 2014 at 2:50 PM, Amit Kapila <amit.kapila16@gmail.com> wrote:
I have extended test (contrib) module dsm_demo such that now user
can specify during dsm_demo_create the lifespan of segment.Applied dsm_keep_segment_v1.patch and dsm_demo_v1.patch.
Got the following warning when I tried above example:postgres=# select dsm_demo_create('this message is from session-new', 1);
WARNING: dynamic shared memory leak: segment 1402373971 still referenced
WARNING: dynamic shared memory leak: segment 1402373971 still referenced
dsm_demo_create
-----------------
1402373971
(1 row)
Thanks for verification.
The reason is that resource owner has reference to segment till commit time
which leads to this warning. The fix is to remove reference from
resource owner when user calls this new API dsm_keep_segment, similar
to what currently dsm_keep_mapping does.
Find the updated patch to fix this problem attached with this mail.
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com
Attachments:
dsm_keep_segment_v2.patchapplication/octet-stream; name=dsm_keep_segment_v2.patchDownload
diff --git a/src/backend/storage/ipc/dsm.c b/src/backend/storage/ipc/dsm.c
index 327d685..130bd39 100644
--- a/src/backend/storage/ipc/dsm.c
+++ b/src/backend/storage/ipc/dsm.c
@@ -42,6 +42,7 @@
#include "utils/guc.h"
#include "utils/memutils.h"
#include "utils/resowner_private.h"
+#include "postmaster/postmaster.h"
#define PG_DYNSHMEM_STATE_FILE PG_DYNSHMEM_DIR "/state"
#define PG_DYNSHMEM_NEW_STATE_FILE PG_DYNSHMEM_DIR "/state.new"
@@ -886,6 +887,58 @@ dsm_keep_mapping(dsm_segment *seg)
}
/*
+ * Keep a dynamic shared memory segment until end of postmaster.
+ *
+ * By default, segments are owned by the current resource owner, which
+ * typically means they stick around for the duration of the current query
+ * only.
+ */
+void
+dsm_keep_segment(dsm_segment *seg)
+{
+ /*
+ * Bump reference count for this segment in shared memory. This will
+ * ensure that even if there is no session which is attached to this
+ * segment, it will stay till postmaster lifetime. For windows, we
+ * need to duplicate segment handle for postmaster process, as it
+ * automatically drops segment if all referring sessions end.
+ */
+ LWLockAcquire(DynamicSharedMemoryControlLock, LW_EXCLUSIVE);
+ dsm_control->item[seg->control_slot].refcnt++;
+ LWLockRelease(DynamicSharedMemoryControlLock);
+
+#ifdef WIN32
+ {
+ HANDLE hmap2;
+
+ if (!DuplicateHandle(GetCurrentProcess(),
+ seg->impl_private,
+ PostmasterHandle,
+ &hmap2,
+ 0,
+ FALSE,
+ DUPLICATE_SAME_ACCESS))
+ {
+ char name[64];
+
+ snprintf(name, 64, "Global/PostgreSQL.%u", seg->handle);
+ _dosmaperr(GetLastError());
+ ereport(ERROR,
+ (errcode_for_file_access(),
+ errmsg("could not retain segment \"%s\" for postmaster lifetime: %m",
+ name)));
+ }
+ }
+#endif
+
+ if (seg->resowner != NULL)
+ {
+ ResourceOwnerForgetDSM(seg->resowner, seg);
+ seg->resowner = NULL;
+ }
+}
+
+/*
* Find an existing mapping for a shared memory segment, if there is one.
*/
dsm_segment *
diff --git a/src/include/storage/dsm.h b/src/include/storage/dsm.h
index 71901bf..16afef3 100644
--- a/src/include/storage/dsm.h
+++ b/src/include/storage/dsm.h
@@ -30,6 +30,7 @@ extern void dsm_detach(dsm_segment *seg);
/* Resource management functions. */
extern void dsm_keep_mapping(dsm_segment *seg);
+extern void dsm_keep_segment(dsm_segment *seg);
extern dsm_segment *dsm_find_mapping(dsm_handle h);
/* Informational functions. */
On Tue, Jan 28, 2014 at 1:41 PM, Amit Kapila <amit.kapila16@gmail.com> wrote:
On Mon, Jan 27, 2014 at 7:48 PM, Amit Langote <amitlangote09@gmail.com> wrote:
On Mon, Jan 13, 2014 at 2:50 PM, Amit Kapila <amit.kapila16@gmail.com> wrote:
I have extended test (contrib) module dsm_demo such that now user
can specify during dsm_demo_create the lifespan of segment.Applied dsm_keep_segment_v1.patch and dsm_demo_v1.patch.
Got the following warning when I tried above example:postgres=# select dsm_demo_create('this message is from session-new', 1);
WARNING: dynamic shared memory leak: segment 1402373971 still referenced
WARNING: dynamic shared memory leak: segment 1402373971 still referenced
dsm_demo_create
-----------------
1402373971
(1 row)Thanks for verification.
The reason is that resource owner has reference to segment till commit time
which leads to this warning. The fix is to remove reference from
resource owner when user calls this new API dsm_keep_segment, similar
to what currently dsm_keep_mapping does.Find the updated patch to fix this problem attached with this mail.
Thanks, the warnings are gone.
--
Amit
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Hello,
Currently there is no way user can keep the dsm
segments if he wants for postmaster lifetime, so I
have exposed a new API dsm_keep_segment()
to implement the same.
I had a short look on this patch.
- DSM implimentation seems divided into generic part (dsm.c) and
platform dependent part(dsm_impl.c). This dsm_keep_segment
puts WIN32 specific part directly into dms.c. I suppose it'd
be better defining DSM_OP_KEEP_SEGMENT and calling dms_impl_op
from dms_keep_segment, or something.
- Repeated calling of dsm_keep_segment even from different
backends creates new (orphan) handles as many as it is called.
Simplly invoking this function in some of extensions intending
to stick segments might results in so many orphan
handles. Something to curb that situation would be needed.
- "Global/PostgreSQL.%u" is the same literal constant with that
occurred in dsm_impl_windows. It should be defined as a
constant (or a macro).
- dms_impl_windows uses errcode_for_dynamic_shared_memory() for
ereport and it finally falls down to
errcode_for_file_access(). I think it is preferable, maybe.
The specs and need for this API is already discussed
in thread:
/messages/by-id/CA+TgmoaKoGuJQbEdGeYKYSXud9EAidqx77J2_HXzRgFo3Hr46A@mail.gmail.comI had used dsm_demo (hacked it a bit) module used
during initial tests for dsm API's to verify the working on
Windows. So one idea could be that I can extend
that module to use this new API, so that it can be tested
by others as well or if you have any other better way, please
do let me know.
I'll run on windows sooner:-)
As the discussion about its specs and need is already
discussed in above mentioned thread, so I will upload
this patch to CF unless there is any Objection.
regards,
--
Kyotaro Horiguchi
NTT Open Source Software Center
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Tue, Jan 28, 2014 at 6:12 AM, Kyotaro HORIGUCHI
<horiguchi.kyotaro@lab.ntt.co.jp> wrote:
I had a short look on this patch.
- DSM implimentation seems divided into generic part (dsm.c) and
platform dependent part(dsm_impl.c). This dsm_keep_segment
puts WIN32 specific part directly into dms.c. I suppose it'd
be better defining DSM_OP_KEEP_SEGMENT and calling dms_impl_op
from dms_keep_segment, or something.
That might not be a very good fit, but maybe there should be a
separate function exposed by dsm_impl.c to do the
implementation-dependent part of the work; it would be a no-op except
on Windows.
- Repeated calling of dsm_keep_segment even from different
backends creates new (orphan) handles as many as it is called.
Simplly invoking this function in some of extensions intending
to stick segments might results in so many orphan
handles. Something to curb that situation would be needed.
I don't really think this is a problem. Let's just document that
dsm_keep_segment() should not be called more than once per segment.
--
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
On Tue, Jan 28, 2014 at 4:42 PM, Kyotaro HORIGUCHI
<horiguchi.kyotaro@lab.ntt.co.jp> wrote:
Hello,
Currently there is no way user can keep the dsm
segments if he wants for postmaster lifetime, so I
have exposed a new API dsm_keep_segment()
to implement the same.I had a short look on this patch.
Thanks.
- DSM implimentation seems divided into generic part (dsm.c) and
platform dependent part(dsm_impl.c). This dsm_keep_segment
puts WIN32 specific part directly into dms.c. I suppose it'd
be better defining DSM_OP_KEEP_SEGMENT and calling dms_impl_op
from dms_keep_segment, or something.- Repeated calling of dsm_keep_segment even from different
backends creates new (orphan) handles as many as it is called.
Simplly invoking this function in some of extensions intending
to stick segments might results in so many orphan
handles. Something to curb that situation would be needed.
I think the right way to fix above 2 comments is as suggested by Robert.
- "Global/PostgreSQL.%u" is the same literal constant with that
occurred in dsm_impl_windows. It should be defined as a
constant (or a macro).- dms_impl_windows uses errcode_for_dynamic_shared_memory() for
ereport and it finally falls down to
errcode_for_file_access(). I think it is preferable, maybe.
Okay, will take care of these in new version after your verification
on Windows.
The specs and need for this API is already discussed
in thread:
/messages/by-id/CA+TgmoaKoGuJQbEdGeYKYSXud9EAidqx77J2_HXzRgFo3Hr46A@mail.gmail.comI had used dsm_demo (hacked it a bit) module used
during initial tests for dsm API's to verify the working on
Windows. So one idea could be that I can extend
that module to use this new API, so that it can be tested
by others as well or if you have any other better way, please
do let me know.I'll run on windows sooner:-)
Please update me once the verification is done on windows.
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Hello, I've managed to reconstruct windows build environment and
tried to run the previous patch.
====================
- DSM implimentation seems divided into generic part (dsm.c) and
platform dependent part(dsm_impl.c). This dsm_keep_segment
puts WIN32 specific part directly into dms.c. I suppose it'd
be better defining DSM_OP_KEEP_SEGMENT and calling dms_impl_op
from dms_keep_segment, or something.- Repeated calling of dsm_keep_segment even from different
backends creates new (orphan) handles as many as it is called.
Simplly invoking this function in some of extensions intending
to stick segments might results in so many orphan
handles. Something to curb that situation would be needed.I think the right way to fix above 2 comments is as suggested by Robert.
Fine. I have no objection on that way.
- "Global/PostgreSQL.%u" is the same literal constant with that
occurred in dsm_impl_windows. It should be defined as a
constant (or a macro).- dms_impl_windows uses errcode_for_dynamic_shared_memory() for
ereport and it finally falls down to
errcode_for_file_access(). I think it is preferable, maybe.Okay, will take care of these in new version after your verification
on Windows.
I will apologize in advance for probably silly questions but I
have two problems.
====
Server was crashed by dsm_demo_read on my test environment but
unfortunately the cause is still uncertain for me.
| LOG: server process (PID 19440) was terminated by exception 0xC0000005
| DETAIL: Failed process was running: select dsm_demo_read(4294967297);
| HINT: See C include file "ntstatus.h" for a description of the hexadecimal value.
| LOG: terminating any other active server processes
0xC0000005 is ACCESS_VIOLATION. The crash occurred at aset.c:853
| /* Try to allocate it */
| block = (AllocBlock) malloc(blksize);
Where blksize is 55011304... It's sasier to investigate still
more if I could step into functions in the dynamic loaded module,
but VC2008 IDE skips over the function body:-( Do you have any
idea how I can step into there? My environment is VC2008 Express/
Win7-64. Step-into bounces back at the line where function
definition.
| PG_FUNCTION_INFO_V1(dsm_demo_create);
In contrast, dsm_demo_create doesn't crash and seems to return
sane vaule.
| =# select dsm_demo_create('hoge', 100);
| dsm_demo_create
| -----------------
| 4294967297
===
And the another problem is perhaps not the issue of this module
but it happened for me.
| =# create extension dsm_demo;
| ERROR: could not find function "dsm_demo_create" in file "c:/pgsql/lib/dsm_demo.dll"
I've found that generated dll file exports the function with the
names following after some investigation.
| ...\Debug>dumpbin /EXPORTS dsm_demo.dll
| Microsoft (R) COFF/PE Dumper Version 9.00.21022.08
| Copyright (C) Microsoft Corporation. All rights reserved.
(snipped)
| 1 0 0001100A Pg_magic_func = @ILT+5(_Pg_magic_func)
| 2 1 00011118 pg_finfo_dsm_demo_create = @ILT+275(_pg_finfo_dsm_demo
| _create)
| 3 2 000110AF pg_finfo_dsm_demo_read = @ILT+170(_pg_finfo_dsm_demo_r
Then explicitly designating the function name in 'CREATE
FUNCTION' in dsm_demo--1.0.sql stops the complaint. I might did
something wrong in setting up build environment for this dll but
I have no idea which would cause this kind of trouble..
| CREATE FUNCTION dsm_demo_create(pg_catalog.text, pg_catalog.int4 default 0)
| RETURNS pg_catalog.int8 STRICT
| AS 'MODULE_PATHNAME', 'pg_finfo_dsm_demo_create'
| LANGUAGE C;
Do you have any idea for this?
regards,
--
Kyotaro Horiguchi
NTT Open Source Software Center
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Fri, Jan 31, 2014 at 1:35 PM, Kyotaro HORIGUCHI
<horiguchi.kyotaro@lab.ntt.co.jp> wrote:
Hello, I've managed to reconstruct windows build environment and
tried to run the previous patch.
Thanks.
I will apologize in advance for probably silly questions but I
have two problems.
I think both the problems are related and the reason is that dsm_demo.dll
is not built properly.
Let us first try to solve your second problem, because I think if
that is solved, you will not face problem-1.
====
Server was crashed by dsm_demo_read on my test environment but
unfortunately the cause is still uncertain for me.| LOG: server process (PID 19440) was terminated by exception 0xC0000005
| DETAIL: Failed process was running: select dsm_demo_read(4294967297);
| HINT: See C include file "ntstatus.h" for a description of the hexadecimal value.
| LOG: terminating any other active server processes0xC0000005 is ACCESS_VIOLATION. The crash occurred at aset.c:853
| /* Try to allocate it */
| block = (AllocBlock) malloc(blksize);Where blksize is 55011304... It's sasier to investigate still
more if I could step into functions in the dynamic loaded module,
but VC2008 IDE skips over the function body:-( Do you have any
idea how I can step into there?
It is because dsm_demo.dll is not built properly.
My environment is VC2008 Express/
Win7-64. Step-into bounces back at the line where function
definition.| PG_FUNCTION_INFO_V1(dsm_demo_create);
In contrast, dsm_demo_create doesn't crash and seems to return
sane vaule.| =# select dsm_demo_create('hoge', 100);
| dsm_demo_create
| -----------------
| 4294967297
It should not be success, because valid value for second argument is
0 or 1, but you have passed 100. Again here the reason could be
dsm_demo.dll is not built properly.
===
And the another problem is perhaps not the issue of this module
but it happened for me.| =# create extension dsm_demo;
| ERROR: could not find function "dsm_demo_create" in file "c:/pgsql/lib/dsm_demo.dll"I've found that generated dll file exports the function with the
names following after some investigation.| ...\Debug>dumpbin /EXPORTS dsm_demo.dll
| Microsoft (R) COFF/PE Dumper Version 9.00.21022.08
| Copyright (C) Microsoft Corporation. All rights reserved.
(snipped)
| 1 0 0001100A Pg_magic_func = @ILT+5(_Pg_magic_func)
| 2 1 00011118 pg_finfo_dsm_demo_create = @ILT+275(_pg_finfo_dsm_demo
| _create)
| 3 2 000110AF pg_finfo_dsm_demo_read = @ILT+170(_pg_finfo_dsm_demo_r
When I did dumpbin, I get following:
ordinal hint RVA name
1 0 00001000 Pg_magic_func = Pg_magic_func
2 1 00001030 dsm_demo_create = dsm_demo_create
3 2 00001230 dsm_demo_read = dsm_demo_read
4 3 00001010 pg_finfo_dsm_demo_create = pg_finfo_dsm_demo_create
5 4 00001020 pg_finfo_dsm_demo_read = pg_finfo_dsm_demo_read
There is a dsm_demo.def file which has below symbols
EXPORTS
Pg_magic_func
dsm_demo_create
dsm_demo_read
pg_finfo_dsm_demo_create
pg_finfo_dsm_demo_read
Could you please once check if you have same exports in your
dsm_demo.def
Unless the above 2 things are not same in your env., it can fail
in multiple ways. Let us first try to see why you are not getting above
symbols.
One more thing, can you please once check if
Debug\Postgres\postgres.def contains symbol dsm_keep_segment.
It might be the case that build of postgres in your m/c doesn't have
dsm_keep_segment and then dsm_demo built based on such
postgres will not be proper. Let me know your finding about this?
Then explicitly designating the function name in 'CREATE
FUNCTION' in dsm_demo--1.0.sql stops the complaint.
You should not do that, certainly there is problem in you build
Do you have any idea for this?
I use Visual studio to build in my environment.
How you are setting up?
Can you once do clean build after applying both(dsm_keep_segment
and dsm_demo) the patches.
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Hello, Now I got workable dll thanks for your advice.
I think both the problems are related and the reason is that dsm_demo.dll
is not built properly.
Let us first try to solve your second problem, because I think if
that is solved, you will not face problem-1.
Thank you for kindness. I got the situation after successfully
getting correct dll by using .def file after your advice. cl
needs __declspec(dllexport) in the symbol definitions to reveal
them externally, without using .def file.
PostgreSQL platform(?) seems offering a macro PGDLLEXPORT for
such use. I suppose this should be used in extension module dlls
to expose symbols, like this,
- void _PG_init(void);
- Datum dsm_demo_create(PG_FUNCTION_ARGS);
- Datum dsm_demo_read(PG_FUNCTION_ARGS);
===
+ PGDLLEXPORT void _PG_init(void);
+ PGDLLEXPORT Datum dsm_demo_create(PG_FUNCTION_ARGS);
+ PGDLLEXPORT Datum dsm_demo_read(PG_FUNCTION_ARGS);# This hardly seems to be used commonly...
I followed this instruction to make build environemnt,
http://blog.2ndquadrant.com/compiling-postgresql-extensions-visual-studio-windows/
And the change above enables us to build this module without .def file.
regards,
--
Kyotaro Horiguchi
NTT Open Source Software Center
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Tue, Feb 4, 2014 at 12:25 PM, Kyotaro HORIGUCHI
<horiguchi.kyotaro@lab.ntt.co.jp> wrote:
Hello, Now I got workable dll thanks for your advice.
I think both the problems are related and the reason is that dsm_demo.dll
is not built properly.
Let us first try to solve your second problem, because I think if
that is solved, you will not face problem-1.Thank you for kindness. I got the situation after successfully
getting correct dll by using .def file after your advice. cl
needs __declspec(dllexport) in the symbol definitions to reveal
them externally, without using .def file.PostgreSQL platform(?) seems offering a macro PGDLLEXPORT for
such use. I suppose this should be used in extension module dlls
to expose symbols, like this,- void _PG_init(void); - Datum dsm_demo_create(PG_FUNCTION_ARGS); - Datum dsm_demo_read(PG_FUNCTION_ARGS); === + PGDLLEXPORT void _PG_init(void); + PGDLLEXPORT Datum dsm_demo_create(PG_FUNCTION_ARGS); + PGDLLEXPORT Datum dsm_demo_read(PG_FUNCTION_ARGS);# This hardly seems to be used commonly...
Yeah, for functions we mainly believe to export using .def file
only and so is the case for this module.
Anyway this is just a test module so if things works for you by
changing the above way, its fine. However I wonder why its not
generating .def file for you.
I followed this instruction to make build environemnt,
http://blog.2ndquadrant.com/compiling-postgresql-extensions-visual-studio-windows/
And the change above enables us to build this module without .def file.
Okay, you can complete your test in the way with which you are able to
successfully build it.
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Hello, I've understood how this works and seems working as
expected.
Anyway this is just a test module so if things works for you by
changing the above way, its fine. However I wonder why its not
generating .def file for you.
Surely.
Getting back on topic, using dsm_keep_segment, I saw postmaster
keeping the section handle for the dsm segment and any session
ever after the creating session gone off could recall the
segment, unlike dsm_keep_mapping couldn't retain them after end
of the creating session. And this exactly resembles linux version
in behavior including attach failure.
The orphan section handles on postmaster have become a matter of
documentation.
Besides all above, I'd like to see a comment for the win32 code
about the 'DuplicateHandle hack', specifically, description that
the DuplicateHandle pushes the copy of the section handle to the
postmaster so the section can retain for the postmaster lifetime.
By the way I have one additional comment.
All postgres processes already keep a section handle for
'Global/PostgreSQL:<pgdata file path>' aside from dsm. It tells
itself is for the file. On the other hand the names for the dsm
sections are 'Global/PostgreSQL.%d'. This seems a bit unkindly as
they don't tell what they are of. Do you mind changing it to,
say, 'Global/PostgreSQL.dsm.%d' or something like that?
regards,
--
Kyotaro Horiguchi
NTT Open Source Software Center
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Hello, let me say in passing,
However I wonder why its not generating .def file for you.
Is the 'it' is Visual Studio IDE or CL? Mmm, as far as I know
.def file is a stuff that programmers should write by their hands
as a matter of course.
I've found no way to automatically generate .def files.. (However
itself would be no matter.)
regards,
--
Kyotaro Horiguchi
NTT Open Source Software Center
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Thu, Feb 6, 2014 at 4:10 PM, Kyotaro HORIGUCHI
<horiguchi.kyotaro@lab.ntt.co.jp> wrote:
Hello, let me say in passing,
However I wonder why its not generating .def file for you.
Is the 'it' is Visual Studio IDE or CL? Mmm, as far as I know
.def file is a stuff that programmers should write by their hands
as a matter of course.
Using MSVC.
We have gendef.pl which can do it.
Example in Postgres project properties, in
Configuration Properties->Build Events->Pre-Link Event, there
is a Command Line like below which can do the required work.
perl src\tools\msvc\gendef.pl Debug\postgres x64
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Thu, Feb 6, 2014 at 3:42 PM, Kyotaro HORIGUCHI
<horiguchi.kyotaro@lab.ntt.co.jp> wrote:
Hello, I've understood how this works and seems working as
expected.Anyway this is just a test module so if things works for you by
changing the above way, its fine. However I wonder why its not
generating .def file for you.Surely.
Getting back on topic, using dsm_keep_segment, I saw postmaster
keeping the section handle for the dsm segment and any session
ever after the creating session gone off could recall the
segment, unlike dsm_keep_mapping couldn't retain them after end
of the creating session. And this exactly resembles linux version
in behavior including attach failure.The orphan section handles on postmaster have become a matter of
documentation.Besides all above, I'd like to see a comment for the win32 code
about the 'DuplicateHandle hack', specifically, description that
the DuplicateHandle pushes the copy of the section handle to the
postmaster so the section can retain for the postmaster lifetime.
Thanks. I shall handle all comments raised by you and send
an updated patch tomorrow.
By the way I have one additional comment.
All postgres processes already keep a section handle for
'Global/PostgreSQL:<pgdata file path>' aside from dsm. It tells
itself is for the file. On the other hand the names for the dsm
sections are 'Global/PostgreSQL.%d'. This seems a bit unkindly as
they don't tell what they are of. Do you mind changing it to,
say, 'Global/PostgreSQL.dsm.%d' or something like that?
I am not sure if there is any benefit of changing the name as it
is used for identification of segment internally and it is uniquely
identified by segment identifier (segment handle), also I think
something similar is use for posix implementation in
dsm_impl_posix(), so we might need to change that as well.
Also as this name is not introduced by this patch, so I think
it will better to keep this as note to committer for this patch and
if there is an agreement for changing it, I will update the patch.
Whats your opinion?
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Thu, Feb 6, 2014 at 3:42 PM, Kyotaro HORIGUCHI
<horiguchi.kyotaro@lab.ntt.co.jp> wrote:Hello, I've understood how this works and seems working as
expected.The orphan section handles on postmaster have become a matter of
documentation.
I had explained this in function header of dsm_keep_segment().
Besides all above, I'd like to see a comment for the win32 code
about the 'DuplicateHandle hack', specifically, description that
the DuplicateHandle pushes the copy of the section handle to the
postmaster so the section can retain for the postmaster lifetime.
I had added a new function in dsm_impl.c for platform specific
handling and explained about new behaviour in function header.
- "Global/PostgreSQL.%u" is the same literal constant with that
occurred in dsm_impl_windows. It should be defined as a
constant (or a macro).
Changed to hash define.
- dms_impl_windows uses errcode_for_dynamic_shared_memory() for
ereport and it finally falls down to
errcode_for_file_access(). I think it is preferable, maybe
Changed as per suggestion.
Please find new version of patch attached with this mail containing
above changes.
Thanks for review.
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com
Attachments:
dsm_keep_segment_v3.patchapplication/octet-stream; name=dsm_keep_segment_v3.patchDownload
diff --git a/src/backend/storage/ipc/dsm.c b/src/backend/storage/ipc/dsm.c
index 327d685..efbd2dd 100644
--- a/src/backend/storage/ipc/dsm.c
+++ b/src/backend/storage/ipc/dsm.c
@@ -886,6 +886,40 @@ dsm_keep_mapping(dsm_segment *seg)
}
/*
+ * Keep a dynamic shared memory segment until end of postmaster.
+ *
+ * This function should not be called more than once per segment,
+ * calling it more number of times will create unnecessary handles
+ * which will consume more memory for no good reason. Though this
+ * behaviour is specific to windows, there is no real benefit for
+ * calling it more than once on other platforms either.
+ *
+ * By default, segments are owned by the current resource owner,
+ * which typically means they stick around for the duration of the
+ * current query only.
+ */
+void
+dsm_keep_segment(dsm_segment *seg)
+{
+ /*
+ * Bump reference count for this segment in shared memory. This will
+ * ensure that even if there is no session which is attached to this
+ * segment, it will stay till postmaster lifetime.
+ */
+ LWLockAcquire(DynamicSharedMemoryControlLock, LW_EXCLUSIVE);
+ dsm_control->item[seg->control_slot].refcnt++;
+ LWLockRelease(DynamicSharedMemoryControlLock);
+
+ dsm_copy_impl_handle(seg->impl_private, seg->handle);
+
+ if (seg->resowner != NULL)
+ {
+ ResourceOwnerForgetDSM(seg->resowner, seg);
+ seg->resowner = NULL;
+ }
+}
+
+/*
* Find an existing mapping for a shared memory segment, if there is one.
*/
dsm_segment *
diff --git a/src/backend/storage/ipc/dsm_impl.c b/src/backend/storage/ipc/dsm_impl.c
index a8d8a64..b6bc93a 100644
--- a/src/backend/storage/ipc/dsm_impl.c
+++ b/src/backend/storage/ipc/dsm_impl.c
@@ -67,6 +67,7 @@
#include "storage/fd.h"
#include "utils/guc.h"
#include "utils/memutils.h"
+#include "postmaster/postmaster.h"
#ifdef USE_DSM_POSIX
static bool dsm_impl_posix(dsm_op op, dsm_handle handle, Size request_size,
@@ -113,6 +114,8 @@ int dynamic_shared_memory_type;
/* Size of buffer to be used for zero-filling. */
#define ZBUFFER_SIZE 8192
+#define SEGMENT_NAME_PREFIX "Global/PostgreSQL"
+
/*------
* Perform a low-level shared memory operation in a platform-specific way,
* as dictated by the selected implementation. Each implementation is
@@ -635,7 +638,7 @@ dsm_impl_windows(dsm_op op, dsm_handle handle, Size request_size,
* convention similar to main shared memory. We can change here once
* issue mentioned in GetSharedMemName is resolved.
*/
- snprintf(name, 64, "Global/PostgreSQL.%u", handle);
+ snprintf(name, 64, "%s.%u", SEGMENT_NAME_PREFIX, handle);
/*
* Handle teardown cases. Since Windows automatically destroys the object
@@ -982,6 +985,48 @@ dsm_impl_mmap(dsm_op op, dsm_handle handle, Size request_size,
}
#endif
+#ifdef WIN32
+/*
+ * The purpose of this function is to duplicate segment handle for
+ * postmaster process, as it automatically drops segment if all
+ * referring sessions end. Copying segment handle for postmaster
+ * process ensures that it will retain the segment for postmaster
+ * lifetime. This is a typical behavior for windows, other platforms
+ * don't need to make any such copy.
+ */
+void dsm_copy_impl_handle(void *impl_private, dsm_handle handle)
+{
+ HANDLE hmap;
+
+ if (!DuplicateHandle(GetCurrentProcess(),
+ impl_private,
+ PostmasterHandle,
+ &hmap,
+ 0,
+ FALSE,
+ DUPLICATE_SAME_ACCESS))
+ {
+ char name[64];
+
+ snprintf(name, 64, "%s.%u", SEGMENT_NAME_PREFIX, handle);
+ _dosmaperr(GetLastError());
+ ereport(ERROR,
+ (errcode_for_dynamic_shared_memory(),
+ errmsg("could not retain segment \"%s\" for postmaster lifetime: %m",
+ name)));
+ }
+}
+#else
+/*
+ * For non-windows platform, this function is no-op.
+ */
+void dsm_copy_impl_handle(void *impl_private, dsm_handle handle)
+{
+ /* no-op */
+}
+
+#endif
+
static int
errcode_for_dynamic_shared_memory()
{
diff --git a/src/include/storage/dsm.h b/src/include/storage/dsm.h
index 71901bf..16afef3 100644
--- a/src/include/storage/dsm.h
+++ b/src/include/storage/dsm.h
@@ -30,6 +30,7 @@ extern void dsm_detach(dsm_segment *seg);
/* Resource management functions. */
extern void dsm_keep_mapping(dsm_segment *seg);
+extern void dsm_keep_segment(dsm_segment *seg);
extern dsm_segment *dsm_find_mapping(dsm_handle h);
/* Informational functions. */
diff --git a/src/include/storage/dsm_impl.h b/src/include/storage/dsm_impl.h
index f2d0c64..bfed25d 100644
--- a/src/include/storage/dsm_impl.h
+++ b/src/include/storage/dsm_impl.h
@@ -72,4 +72,7 @@ extern bool dsm_impl_op(dsm_op op, dsm_handle handle, Size request_size,
/* Some implementations cannot resize segments. Can this one? */
extern bool dsm_impl_can_resize(void);
+/* create a copy of implementation specific handle in postmaster process. */
+extern void dsm_copy_impl_handle(void *impl_private, dsm_handle handle);
+
#endif /* DSM_IMPL_H */
Hello,
Please find new version of patch attached with this mail containing
above changes.
This patch applies cleanly on current HEAD and build completed
successfully on both Windows and Linux. (but master needed to be
rewinded to some time ago for some compile errors.)
This works correctly as before.
Finally before send to commiters, would you mind changing the
name of the segment "Global/PostgreSQL.%u" as
'Global/PostgreSQL.dsm.%u" or something mentioned in the last my
email? However, it is a bit different thing from this patch so
I have no intention to compel to do the changing.
The orphan section handles on postmaster have become a matter of
documentation.I had explained this in function header of dsm_keep_segment().
The comment satisfies me. Thank you.
I had added a new function in dsm_impl.c for platform specific
handling and explained about new behaviour in function header.
This seems quite clear for me.
- "Global/PostgreSQL.%u" is the same literal constant with that
occurred in dsm_impl_windows. It should be defined as a
constant (or a macro).Changed to hash define.
Thank you.
- dms_impl_windows uses errcode_for_dynamic_shared_memory() for
ereport and it finally falls down to
errcode_for_file_access(). I think it is preferable, maybeChanged as per suggestion.
I saw it done.
regards,
--
Kyotaro Horiguchi
NTT Open Source Software Center
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Wed, Feb 12, 2014 at 2:02 PM, Kyotaro HORIGUCHI
<horiguchi.kyotaro@lab.ntt.co.jp> wrote:
Hello,
Please find new version of patch attached with this mail containing
above changes.This patch applies cleanly on current HEAD and build completed
successfully on both Windows and Linux. (but master needed to be
rewinded to some time ago for some compile errors.)This works correctly as before.
Finally before send to commiters, would you mind changing the
name of the segment "Global/PostgreSQL.%u" as
'Global/PostgreSQL.dsm.%u" or something mentioned in the last my
email?
Actually in that case, to maintain consistency we need to change even in
function dsm_impl_posix() which uses segment name as "/PostgreSQL.%u".
For windows, we have added "Global" to "/PostgreSQL.%u", as it is
mandate by windows spec.
To be honest, I see no harm in changing the name as per your suggestion,
as it can improve segment naming for dynamic shared memory segments,
however there is no clear problem with current name as well, so I don't
want to change in places this patch has no relation.
I think best thing to do here is to put it as Notes To Committer, something
like:
Some suggestions for Committer to consider:
"Change the name of dsm segments from .. to .."
In general, what I see is that they consider all discussion in thread, but
putting some special notes like above will reduce the chance of getting
overlooked by them. I have done as a reviewer previously and it worked
well.
However, it is a bit different thing from this patch so
I have no intention to compel to do the changing.
Thanks to you for understanding my position.
Thanks for reviewing the patch so carefully, especially Windows part
which I think was bit tricky for you to setup.
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Hello, I've marked this patch as 'Ready for committer'.
To be honest, I see no harm in changing the name as per your suggestion,
as it can improve segment naming for dynamic shared memory segments,
however there is no clear problem with current name as well, so I don't
want to change in places this patch has no relation.
Okay, let's go with it as it is.
I think best thing to do here is to put it as Notes To Committer, something
like:
Some suggestions for Committer to consider:
"Change the name of dsm segments from .. to .."
In general, what I see is that they consider all discussion in thread, but
putting some special notes like above will reduce the chance of getting
overlooked by them. I have done as a reviewer previously and it worked
well.
Thank you for the sugestion.
However, it is a bit different thing from this patch so
I have no intention to compel to do the changing.Thanks to you for understanding my position.
Thanks for reviewing the patch so carefully, especially Windows part
which I think was bit tricky for you to setup.
It's my presure and I learned a lot.
regards,
--
Kyotaro Horiguchi
NTT Open Source Software Center
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Thank you for letting me know of that.
Using MSVC.
We have gendef.pl which can do it.
Mmm.. My eyes skipped over it. Everything became clear for
me. Thank you.
Example in Postgres project properties, in
Configuration Properties->Build Events->Pre-Link Event, there
is a Command Line like below which can do the required work.
perl src\tools\msvc\gendef.pl Debug\postgres x64
--
Kyotaro Horiguchi
NTT Open Source Software Center
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Sat, Feb 8, 2014 at 2:31 AM, Amit Kapila <amit.kapila16@gmail.com> wrote:
On Thu, Feb 6, 2014 at 3:42 PM, Kyotaro HORIGUCHI
<horiguchi.kyotaro@lab.ntt.co.jp> wrote:Hello, I've understood how this works and seems working as
expected.The orphan section handles on postmaster have become a matter of
documentation.I had explained this in function header of dsm_keep_segment().
Besides all above, I'd like to see a comment for the win32 code
about the 'DuplicateHandle hack', specifically, description that
the DuplicateHandle pushes the copy of the section handle to the
postmaster so the section can retain for the postmaster lifetime.I had added a new function in dsm_impl.c for platform specific
handling and explained about new behaviour in function header.- "Global/PostgreSQL.%u" is the same literal constant with that
occurred in dsm_impl_windows. It should be defined as a
constant (or a macro).Changed to hash define.
- dms_impl_windows uses errcode_for_dynamic_shared_memory() for
ereport and it finally falls down to
errcode_for_file_access(). I think it is preferable, maybeChanged as per suggestion.
Please find new version of patch attached with this mail containing
above changes.
I took a look at this patch. It seems to me that it doesn't do a very
good job maintaining the abstraction boundary between what the dsm.c
layer knows about and what the dsm_impl.c layer knows about. However,
AFAICS, these problems are purely cosmetic, so I took a crack at
fixing them. I retitled the new implementation-layer function to
dsm_impl_keep_segment(), swapped the order of the arguments for
consistency with other code, adjusted the dsm_impl.c code slightly to
avoid assuming that only the Windows implementation works on Windows
(that's currently true, but we could probably make the mmap
implementation work there as well), and retooled some of the comments
to read better in English. I'm happy with the attached version but
don't have a Windows box to test it there.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
Attachments:
dsm_keep_segment_v4.patchtext/x-patch; charset=US-ASCII; name=dsm_keep_segment_v4.patchDownload
diff --git a/src/backend/storage/ipc/dsm.c b/src/backend/storage/ipc/dsm.c
index 31e592e..c7dc971 100644
--- a/src/backend/storage/ipc/dsm.c
+++ b/src/backend/storage/ipc/dsm.c
@@ -886,6 +886,34 @@ dsm_keep_mapping(dsm_segment *seg)
}
/*
+ * Keep a dynamic shared memory segment until postmaster shutdown.
+ *
+ * This function should not be called more than once per segment;
+ * on Windows, doing so will create unnecessary handles which will
+ * consume system resources to no benefit.
+ */
+void
+dsm_keep_segment(dsm_segment *seg)
+{
+ /*
+ * Bump reference count for this segment in shared memory. This will
+ * ensure that even if there is no session which is attached to this
+ * segment, it will remain until postmaster shutdown.
+ */
+ LWLockAcquire(DynamicSharedMemoryControlLock, LW_EXCLUSIVE);
+ dsm_control->item[seg->control_slot].refcnt++;
+ LWLockRelease(DynamicSharedMemoryControlLock);
+
+ dsm_impl_keep_segment(seg->handle, seg->impl_private);
+
+ if (seg->resowner != NULL)
+ {
+ ResourceOwnerForgetDSM(seg->resowner, seg);
+ seg->resowner = NULL;
+ }
+}
+
+/*
* Find an existing mapping for a shared memory segment, if there is one.
*/
dsm_segment *
diff --git a/src/backend/storage/ipc/dsm_impl.c b/src/backend/storage/ipc/dsm_impl.c
index a8d8a64..221044a 100644
--- a/src/backend/storage/ipc/dsm_impl.c
+++ b/src/backend/storage/ipc/dsm_impl.c
@@ -67,6 +67,7 @@
#include "storage/fd.h"
#include "utils/guc.h"
#include "utils/memutils.h"
+#include "postmaster/postmaster.h"
#ifdef USE_DSM_POSIX
static bool dsm_impl_posix(dsm_op op, dsm_handle handle, Size request_size,
@@ -113,6 +114,8 @@ int dynamic_shared_memory_type;
/* Size of buffer to be used for zero-filling. */
#define ZBUFFER_SIZE 8192
+#define SEGMENT_NAME_PREFIX "Global/PostgreSQL"
+
/*------
* Perform a low-level shared memory operation in a platform-specific way,
* as dictated by the selected implementation. Each implementation is
@@ -635,7 +638,7 @@ dsm_impl_windows(dsm_op op, dsm_handle handle, Size request_size,
* convention similar to main shared memory. We can change here once
* issue mentioned in GetSharedMemName is resolved.
*/
- snprintf(name, 64, "Global/PostgreSQL.%u", handle);
+ snprintf(name, 64, "%s.%u", SEGMENT_NAME_PREFIX, handle);
/*
* Handle teardown cases. Since Windows automatically destroys the object
@@ -982,6 +985,45 @@ dsm_impl_mmap(dsm_op op, dsm_handle handle, Size request_size,
}
#endif
+/*
+ * Implementation-specific actions that must be performed when a segment
+ * is to be preserved until postmaster shutdown.
+ *
+ * Except on Windows, we don't need to do anything at all. But since Windows
+ * cleans up segments automatically when no references remain, we duplicate
+ * the segment handle into the postmaster process. The postmaster needn't
+ * do anything to receive the handle; Windows transfers it automatically.
+ */
+void
+dsm_impl_keep_segment(dsm_handle handle, void *impl_private)
+{
+ switch (dynamic_shared_memory_type)
+ {
+#ifdef USE_DSM_WINDOWS
+ case DSM_IMPL_WINDOWS:
+ {
+ HANDLE hmap;
+
+ if (!DuplicateHandle(GetCurrentProcess(), impl_private,
+ PostmasterHandle, &hmap, 0, FALSE,
+ DUPLICATE_SAME_ACCESS))
+ {
+ char name[64];
+
+ snprintf(name, 64, "%s.%u", SEGMENT_NAME_PREFIX, handle);
+ _dosmaperr(GetLastError());
+ ereport(ERROR,
+ (errcode_for_dynamic_shared_memory(),
+ errmsg("could not duplicate handle: %m")));
+ }
+ break;
+ }
+#endif
+ default:
+ break;
+ }
+}
+
static int
errcode_for_dynamic_shared_memory()
{
diff --git a/src/include/storage/dsm.h b/src/include/storage/dsm.h
index 46d3cbd..03dd767 100644
--- a/src/include/storage/dsm.h
+++ b/src/include/storage/dsm.h
@@ -30,6 +30,7 @@ extern void dsm_detach(dsm_segment *seg);
/* Resource management functions. */
extern void dsm_keep_mapping(dsm_segment *seg);
+extern void dsm_keep_segment(dsm_segment *seg);
extern dsm_segment *dsm_find_mapping(dsm_handle h);
/* Informational functions. */
diff --git a/src/include/storage/dsm_impl.h b/src/include/storage/dsm_impl.h
index f2d0c64..fda5514 100644
--- a/src/include/storage/dsm_impl.h
+++ b/src/include/storage/dsm_impl.h
@@ -72,4 +72,7 @@ extern bool dsm_impl_op(dsm_op op, dsm_handle handle, Size request_size,
/* Some implementations cannot resize segments. Can this one? */
extern bool dsm_impl_can_resize(void);
+/* Implementation-dependent actions required to keep segment until shudown. */
+extern void dsm_impl_keep_segment(dsm_handle handle, void *impl_private);
+
#endif /* DSM_IMPL_H */
On Mon, Mar 10, 2014 at 8:18 PM, Robert Haas <robertmhaas@gmail.com> wrote:
I took a look at this patch. It seems to me that it doesn't do a very
good job maintaining the abstraction boundary between what the dsm.c
layer knows about and what the dsm_impl.c layer knows about. However,
AFAICS, these problems are purely cosmetic, so I took a crack at
fixing them. I retitled the new implementation-layer function to
dsm_impl_keep_segment(), swapped the order of the arguments for
consistency with other code, adjusted the dsm_impl.c code slightly to
avoid assuming that only the Windows implementation works on Windows
(that's currently true, but we could probably make the mmap
implementation work there as well), and retooled some of the comments
to read better in English. I'm happy with the attached version but
don't have a Windows box to test it there.
Thank you for looking into patch. I have verified that attached patch
works fine on Windows.
One observation in new version of patch:
+ {
+ char name[64];
+
+ snprintf(name, 64, "%s.%u", SEGMENT_NAME_PREFIX, handle);
+ _dosmaperr(GetLastError());
+ ereport(ERROR,
+ (errcode_for_dynamic_shared_memory(),
+ errmsg("could not duplicate handle: %m")));
+ }Now, the patch doesn't use segment *name* in errmsg which makes
it and handle passed in function dsm_impl_keep_segment() redundant.
I think its better to use name in errmsg as it is used at other places
in code as well and make the error more meaningful. However if you
feel it is better otherwise, then we should remove not required variable
and parameter in function dsm_impl_keep_segment()
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Mon, Mar 10, 2014 at 9:48 PM, Amit Kapila <amit.kapila16@gmail.com> wrote:
On Mon, Mar 10, 2014 at 8:18 PM, Robert Haas <robertmhaas@gmail.com> wrote:
I took a look at this patch. It seems to me that it doesn't do a very
good job maintaining the abstraction boundary between what the dsm.c
layer knows about and what the dsm_impl.c layer knows about. However,
AFAICS, these problems are purely cosmetic, so I took a crack at
fixing them. I retitled the new implementation-layer function to
dsm_impl_keep_segment(), swapped the order of the arguments for
consistency with other code, adjusted the dsm_impl.c code slightly to
avoid assuming that only the Windows implementation works on Windows
(that's currently true, but we could probably make the mmap
implementation work there as well), and retooled some of the comments
to read better in English. I'm happy with the attached version but
don't have a Windows box to test it there.Thank you for looking into patch. I have verified that attached patch
works fine on Windows.One observation in new version of patch:
+ { + char name[64]; + + snprintf(name, 64, "%s.%u", SEGMENT_NAME_PREFIX, handle); + _dosmaperr(GetLastError()); + ereport(ERROR, + (errcode_for_dynamic_shared_memory(), + errmsg("could not duplicate handle: %m"))); + }
I have updated the patch to change message as below:
errmsg("could not duplicate handle for \"%s\": %m",
name)));
Let me know your suggestions?
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com
Attachments:
dsm_keep_segment_v5.patchapplication/octet-stream; name=dsm_keep_segment_v5.patchDownload
diff --git a/src/backend/storage/ipc/dsm.c b/src/backend/storage/ipc/dsm.c
index 327d685..0841de3 100644
--- a/src/backend/storage/ipc/dsm.c
+++ b/src/backend/storage/ipc/dsm.c
@@ -886,6 +886,34 @@ dsm_keep_mapping(dsm_segment *seg)
}
/*
+ * Keep a dynamic shared memory segment until postmaster shutdown.
+ *
+ * This function should not be called more than once per segment;
+ * on Windows, doing so will create unnecessary handles which will
+ * consume system resources to no benefit.
+ */
+void
+dsm_keep_segment(dsm_segment *seg)
+{
+ /*
+ * Bump reference count for this segment in shared memory. This will
+ * ensure that even if there is no session which is attached to this
+ * segment, it will remain until postmaster shutdown.
+ */
+ LWLockAcquire(DynamicSharedMemoryControlLock, LW_EXCLUSIVE);
+ dsm_control->item[seg->control_slot].refcnt++;
+ LWLockRelease(DynamicSharedMemoryControlLock);
+
+ dsm_impl_keep_segment(seg->handle, seg->impl_private);
+
+ if (seg->resowner != NULL)
+ {
+ ResourceOwnerForgetDSM(seg->resowner, seg);
+ seg->resowner = NULL;
+ }
+}
+
+/*
* Find an existing mapping for a shared memory segment, if there is one.
*/
dsm_segment *
diff --git a/src/backend/storage/ipc/dsm_impl.c b/src/backend/storage/ipc/dsm_impl.c
index a8d8a64..745ff23 100644
--- a/src/backend/storage/ipc/dsm_impl.c
+++ b/src/backend/storage/ipc/dsm_impl.c
@@ -67,6 +67,7 @@
#include "storage/fd.h"
#include "utils/guc.h"
#include "utils/memutils.h"
+#include "postmaster/postmaster.h"
#ifdef USE_DSM_POSIX
static bool dsm_impl_posix(dsm_op op, dsm_handle handle, Size request_size,
@@ -113,6 +114,8 @@ int dynamic_shared_memory_type;
/* Size of buffer to be used for zero-filling. */
#define ZBUFFER_SIZE 8192
+#define SEGMENT_NAME_PREFIX "Global/PostgreSQL"
+
/*------
* Perform a low-level shared memory operation in a platform-specific way,
* as dictated by the selected implementation. Each implementation is
@@ -635,7 +638,7 @@ dsm_impl_windows(dsm_op op, dsm_handle handle, Size request_size,
* convention similar to main shared memory. We can change here once
* issue mentioned in GetSharedMemName is resolved.
*/
- snprintf(name, 64, "Global/PostgreSQL.%u", handle);
+ snprintf(name, 64, "%s.%u", SEGMENT_NAME_PREFIX, handle);
/*
* Handle teardown cases. Since Windows automatically destroys the object
@@ -982,6 +985,46 @@ dsm_impl_mmap(dsm_op op, dsm_handle handle, Size request_size,
}
#endif
+/*
+ * Implementation-specific actions that must be performed when a segment
+ * is to be preserved until postmaster shutdown.
+ *
+ * Except on Windows, we don't need to do anything at all. But since Windows
+ * cleans up segments automatically when no references remain, we duplicate
+ * the segment handle into the postmaster process. The postmaster needn't
+ * do anything to receive the handle; Windows transfers it automatically.
+ */
+void
+dsm_impl_keep_segment(dsm_handle handle, void *impl_private)
+{
+ switch (dynamic_shared_memory_type)
+ {
+#ifdef USE_DSM_WINDOWS
+ case DSM_IMPL_WINDOWS:
+ {
+ HANDLE hmap;
+
+ if (!DuplicateHandle(GetCurrentProcess(), impl_private,
+ PostmasterHandle, &hmap, 0, FALSE,
+ DUPLICATE_SAME_ACCESS))
+ {
+ char name[64];
+
+ snprintf(name, 64, "%s.%u", SEGMENT_NAME_PREFIX, handle);
+ _dosmaperr(GetLastError());
+ ereport(ERROR,
+ (errcode_for_dynamic_shared_memory(),
+ errmsg("could not duplicate handle for \"%s\": %m",
+ name)));
+ }
+ break;
+ }
+#endif
+ default:
+ break;
+ }
+}
+
static int
errcode_for_dynamic_shared_memory()
{
diff --git a/src/include/storage/dsm.h b/src/include/storage/dsm.h
index 71901bf..16afef3 100644
--- a/src/include/storage/dsm.h
+++ b/src/include/storage/dsm.h
@@ -30,6 +30,7 @@ extern void dsm_detach(dsm_segment *seg);
/* Resource management functions. */
extern void dsm_keep_mapping(dsm_segment *seg);
+extern void dsm_keep_segment(dsm_segment *seg);
extern dsm_segment *dsm_find_mapping(dsm_handle h);
/* Informational functions. */
diff --git a/src/include/storage/dsm_impl.h b/src/include/storage/dsm_impl.h
index f2d0c64..fda5514 100644
--- a/src/include/storage/dsm_impl.h
+++ b/src/include/storage/dsm_impl.h
@@ -72,4 +72,7 @@ extern bool dsm_impl_op(dsm_op op, dsm_handle handle, Size request_size,
/* Some implementations cannot resize segments. Can this one? */
extern bool dsm_impl_can_resize(void);
+/* Implementation-dependent actions required to keep segment until shudown. */
+extern void dsm_impl_keep_segment(dsm_handle handle, void *impl_private);
+
#endif /* DSM_IMPL_H */
On Mon, Mar 10, 2014 at 12:44 PM, Amit Kapila <amit.kapila16@gmail.com> wrote:
On Mon, Mar 10, 2014 at 9:48 PM, Amit Kapila <amit.kapila16@gmail.com> wrote:
On Mon, Mar 10, 2014 at 8:18 PM, Robert Haas <robertmhaas@gmail.com> wrote:
I took a look at this patch. It seems to me that it doesn't do a very
good job maintaining the abstraction boundary between what the dsm.c
layer knows about and what the dsm_impl.c layer knows about. However,
AFAICS, these problems are purely cosmetic, so I took a crack at
fixing them. I retitled the new implementation-layer function to
dsm_impl_keep_segment(), swapped the order of the arguments for
consistency with other code, adjusted the dsm_impl.c code slightly to
avoid assuming that only the Windows implementation works on Windows
(that's currently true, but we could probably make the mmap
implementation work there as well), and retooled some of the comments
to read better in English. I'm happy with the attached version but
don't have a Windows box to test it there.Thank you for looking into patch. I have verified that attached patch
works fine on Windows.One observation in new version of patch:
+ { + char name[64]; + + snprintf(name, 64, "%s.%u", SEGMENT_NAME_PREFIX, handle); + _dosmaperr(GetLastError()); + ereport(ERROR, + (errcode_for_dynamic_shared_memory(), + errmsg("could not duplicate handle: %m"))); + }I have updated the patch to change message as below:
errmsg("could not duplicate handle for \"%s\": %m",
name)));Let me know your suggestions?
Looks good, committed. However, I changed it so that
dsm_keep_segment() does not also perform the equivalent of
dsm_keep_mapping(); those are two separate operations.
--
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
On Mon, Mar 10, 2014 at 11:37 PM, Robert Haas <robertmhaas@gmail.com> wrote:
Looks good, committed. However, I changed it so that
dsm_keep_segment() does not also perform the equivalent of
dsm_keep_mapping(); those are two separate operations.
So are you expecting that if some one needs to retain dynamic segment's
till PM lifetime, they should call both dsm_keep_segment() and
dsm_keep_mapping()?
If we don't call both, it can lead to following warning:
postgres=# select dsm_demo_create('this message is from session-new', 1);
WARNING: dynamic shared memory leak: segment 1402373971 still referenced
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Mon, Mar 10, 2014 at 11:26 PM, Amit Kapila <amit.kapila16@gmail.com> wrote:
On Mon, Mar 10, 2014 at 11:37 PM, Robert Haas <robertmhaas@gmail.com> wrote:
Looks good, committed. However, I changed it so that
dsm_keep_segment() does not also perform the equivalent of
dsm_keep_mapping(); those are two separate operations.So are you expecting that if some one needs to retain dynamic segment's
till PM lifetime, they should call both dsm_keep_segment() and
dsm_keep_mapping()?
If they want to keep both the mapping and the segment, yes. But in
general those two things are independent of each other. A process
could want to map the segment and store some data in it, and then it
could want to unmap the segment; and then later the segment could be
mapped again (perhaps from some other backend) to get the data out.
If we don't call both, it can lead to following warning:
postgres=# select dsm_demo_create('this message is from session-new', 1);
WARNING: dynamic shared memory leak: segment 1402373971 still referenced
Well, that's just an artifact of the coding of dsm_demo_create().
Code that doesn't use dsm_keep_mapping() needs to be sure to call
dsm_detach() in the non-error path.
--
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