Re: [HACKERS] Retain dynamic shared memory segments for postmaster lifetime

2014-03-11 Thread Robert Haas
On Mon, Mar 10, 2014 at 11:26 PM, Amit Kapila amit.kapil...@gmail.com wrote:
 On Mon, Mar 10, 2014 at 11:37 PM, Robert Haas robertmh...@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


Re: [HACKERS] Retain dynamic shared memory segments for postmaster lifetime

2014-03-10 Thread Robert Haas
On Sat, Feb 8, 2014 at 2:31 AM, Amit Kapila amit.kapil...@gmail.com wrote:
 On Thu, Feb 6, 2014 at 3:42 PM, Kyotaro HORIGUCHI
 horiguchi.kyot...@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.

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
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_SIZE8192
 
+#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 

Re: [HACKERS] Retain dynamic shared memory segments for postmaster lifetime

2014-03-10 Thread Amit Kapila
On Mon, Mar 10, 2014 at 8:18 PM, Robert Haas robertmh...@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


Re: [HACKERS] Retain dynamic shared memory segments for postmaster lifetime

2014-03-10 Thread Amit Kapila
On Mon, Mar 10, 2014 at 9:48 PM, Amit Kapila amit.kapil...@gmail.com wrote:
 On Mon, Mar 10, 2014 at 8:18 PM, Robert Haas robertmh...@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


dsm_keep_segment_v5.patch
Description: Binary data

-- 
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] Retain dynamic shared memory segments for postmaster lifetime

2014-03-10 Thread Robert Haas
On Mon, Mar 10, 2014 at 12:44 PM, Amit Kapila amit.kapil...@gmail.com wrote:
 On Mon, Mar 10, 2014 at 9:48 PM, Amit Kapila amit.kapil...@gmail.com wrote:
 On Mon, Mar 10, 2014 at 8:18 PM, Robert Haas robertmh...@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


Re: [HACKERS] Retain dynamic shared memory segments for postmaster lifetime

2014-03-10 Thread Amit Kapila
On Mon, Mar 10, 2014 at 11:37 PM, Robert Haas robertmh...@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


Re: [HACKERS] Retain dynamic shared memory segments for postmaster lifetime

2014-02-16 Thread Kyotaro HORIGUCHI
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


Re: [HACKERS] Retain dynamic shared memory segments for postmaster lifetime

2014-02-12 Thread Kyotaro HORIGUCHI
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, maybe
 
 Changed 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


Re: [HACKERS] Retain dynamic shared memory segments for postmaster lifetime

2014-02-12 Thread Amit Kapila
On Wed, Feb 12, 2014 at 2:02 PM, Kyotaro HORIGUCHI
horiguchi.kyot...@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


Re: [HACKERS] Retain dynamic shared memory segments for postmaster lifetime

2014-02-12 Thread Kyotaro HORIGUCHI
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


Re: [HACKERS] Retain dynamic shared memory segments for postmaster lifetime

2014-02-07 Thread Amit Kapila
 On Thu, Feb 6, 2014 at 3:42 PM, Kyotaro HORIGUCHI
 horiguchi.kyot...@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


dsm_keep_segment_v3.patch
Description: Binary data

-- 
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] Retain dynamic shared memory segments for postmaster lifetime

2014-02-06 Thread Kyotaro HORIGUCHI
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


Re: [HACKERS] Retain dynamic shared memory segments for postmaster lifetime

2014-02-06 Thread Kyotaro HORIGUCHI
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


Re: [HACKERS] Retain dynamic shared memory segments for postmaster lifetime

2014-02-06 Thread Amit Kapila
On Thu, Feb 6, 2014 at 4:10 PM, Kyotaro HORIGUCHI
horiguchi.kyot...@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


Re: [HACKERS] Retain dynamic shared memory segments for postmaster lifetime

2014-02-06 Thread Amit Kapila
On Thu, Feb 6, 2014 at 3:42 PM, Kyotaro HORIGUCHI
horiguchi.kyot...@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


Re: [HACKERS] Retain dynamic shared memory segments for postmaster lifetime

2014-02-03 Thread Kyotaro HORIGUCHI
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


Re: [HACKERS] Retain dynamic shared memory segments for postmaster lifetime

2014-02-03 Thread Amit Kapila
On Tue, Feb 4, 2014 at 12:25 PM, Kyotaro HORIGUCHI
horiguchi.kyot...@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


Re: [HACKERS] Retain dynamic shared memory segments for postmaster lifetime

2014-01-31 Thread Kyotaro HORIGUCHI
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 0xC005
| 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

0xC005 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.

| ...\Debugdumpbin /EXPORTS dsm_demo.dll
| Microsoft (R) COFF/PE Dumper Version 9.00.21022.08
| Copyright (C) Microsoft Corporation.  All rights reserved.
(snipped)
|   10 0001100A Pg_magic_func = @ILT+5(_Pg_magic_func)
|   21 0008 pg_finfo_dsm_demo_create = 
@ILT+275(_pg_finfo_dsm_demo
| _create)
|   32 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


Re: [HACKERS] Retain dynamic shared memory segments for postmaster lifetime

2014-01-31 Thread Amit Kapila
On Fri, Jan 31, 2014 at 1:35 PM, Kyotaro HORIGUCHI
horiguchi.kyot...@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 0xC005
 | 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

 0xC005 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.

 | ...\Debugdumpbin /EXPORTS dsm_demo.dll
 | Microsoft (R) COFF/PE Dumper Version 9.00.21022.08
 | Copyright (C) Microsoft Corporation.  All rights reserved.
 (snipped)
 |   10 0001100A Pg_magic_func = @ILT+5(_Pg_magic_func)
 |   21 0008 pg_finfo_dsm_demo_create = 
 @ILT+275(_pg_finfo_dsm_demo
 | _create)
 |   32 000110AF pg_finfo_dsm_demo_read = 
 @ILT+170(_pg_finfo_dsm_demo_r

When I did dumpbin, I get following:
ordinal hint RVA  name

  10 1000 Pg_magic_func = Pg_magic_func
  21 1030 dsm_demo_create = dsm_demo_create
  32 1230 dsm_demo_read = dsm_demo_read
  43 1010 pg_finfo_dsm_demo_create = pg_finfo_dsm_demo_create
  54 1020 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


Re: [HACKERS] Retain dynamic shared memory segments for postmaster lifetime

2014-01-28 Thread Kyotaro HORIGUCHI
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:
 http://www.postgresql.org/message-id/ca+tgmoakogujqbedgeykysxud9eaidqx77j2_hxzrgfo3hr...@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.

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


Re: [HACKERS] Retain dynamic shared memory segments for postmaster lifetime

2014-01-28 Thread Robert Haas
On Tue, Jan 28, 2014 at 6:12 AM, Kyotaro HORIGUCHI
horiguchi.kyot...@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


Re: [HACKERS] Retain dynamic shared memory segments for postmaster lifetime

2014-01-28 Thread Amit Kapila
On Tue, Jan 28, 2014 at 4:42 PM, Kyotaro HORIGUCHI
horiguchi.kyot...@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:
 http://www.postgresql.org/message-id/ca+tgmoakogujqbedgeykysxud9eaidqx77j2_hxzrgfo3hr...@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.

 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


Re: [HACKERS] Retain dynamic shared memory segments for postmaster lifetime

2014-01-27 Thread Amit Langote
On Mon, Jan 27, 2014 at 11:18 PM, Amit Langote amitlangot...@gmail.com wrote:
 On Mon, Jan 13, 2014 at 2:50 PM, Amit Kapila amit.kapil...@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 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
 -
82712

 Session - 2
 -
 select dsm_demo_read(82712);
dsm_demo_read
 
  this message is from session-1
 (1 row)


 Session-1
 \q

 Session-2
 postgres=# select dsm_demo_read(82712);
  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
 -
82712

 Session - 2
 -
 select dsm_demo_read(82712);
dsm_demo_read
 
  this message is from session-1
 (1 row)


 Session-1
 \q

 Session-2
 postgres=# select dsm_demo_read(82712);
  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


Re: [HACKERS] Retain dynamic shared memory segments for postmaster lifetime

2014-01-27 Thread Amit Langote
On Mon, Jan 13, 2014 at 2:50 PM, Amit Kapila amit.kapil...@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 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
 -
82712

 Session - 2
 -
 select dsm_demo_read(82712);
dsm_demo_read
 
  this message is from session-1
 (1 row)


 Session-1
 \q

 Session-2
 postgres=# select dsm_demo_read(82712);
  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
 -
82712

 Session - 2
 -
 select dsm_demo_read(82712);
dsm_demo_read
 
  this message is from session-1
 (1 row)


 Session-1
 \q

 Session-2
 postgres=# select dsm_demo_read(82712);
  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


Re: [HACKERS] Retain dynamic shared memory segments for postmaster lifetime

2014-01-27 Thread Amit Kapila
On Mon, Jan 27, 2014 at 7:48 PM, Amit Langote amitlangot...@gmail.com wrote:
 On Mon, Jan 13, 2014 at 2:50 PM, Amit Kapila amit.kapil...@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


dsm_keep_segment_v2.patch
Description: Binary data

-- 
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] Retain dynamic shared memory segments for postmaster lifetime

2014-01-27 Thread Amit Langote
On Tue, Jan 28, 2014 at 1:41 PM, Amit Kapila amit.kapil...@gmail.com wrote:
 On Mon, Jan 27, 2014 at 7:48 PM, Amit Langote amitlangot...@gmail.com wrote:
 On Mon, Jan 13, 2014 at 2:50 PM, Amit Kapila amit.kapil...@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


Re: [HACKERS] Retain dynamic shared memory segments for postmaster lifetime

2014-01-12 Thread Amit Kapila
On Sun, Jan 12, 2014 at 12:41 PM, Amit Kapila amit.kapil...@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:
 http://www.postgresql.org/message-id/ca+tgmoakogujqbedgeykysxud9eaidqx77j2_hxzrgfo3hr...@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
-
   82712

Session - 2
-
select dsm_demo_read(82712);
   dsm_demo_read

 this message is from session-1
(1 row)


Session-1
\q

Session-2
postgres=# select dsm_demo_read(82712);
 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
-
   82712

Session - 2
-
select dsm_demo_read(82712);
   dsm_demo_read

 this message is from session-1
(1 row)


Session-1
\q

Session-2
postgres=# select dsm_demo_read(82712);
 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


dsm_demo_v1.patch
Description: Binary data

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