[HACKERS] Re: [HACKERS] Re: [HACKERS] Re: [HACKERS] Re: [HACKERS] Windows service is not starting so there’s message in log: FATAL: "could not create shared memory segment “Global/PostgreSQL.851401618

2016-09-22 Thread Amit Kapila
On Fri, Sep 23, 2016 at 1:21 AM, Tom Lane  wrote:
> Robert Haas  writes:
>> On Tue, Sep 20, 2016 at 12:53 PM, Tom Lane  wrote:
>>> I'd be the first to agree that this point is inadequately documented
>>> in the code, but PostmasterRandom should be reserved for its existing
>>> security-related uses, not exposed to the world for (ahem) random other
>>> uses.
>
>> So, we could have dsm_postmaster_startup() seed the random number
>> generator itself, and then let PostmasterRandom() override the seed
>> later.  Like maybe:
>
> Works for me, at least as a temporary solution.  The disturbing thing
> here is that this still only does what we want if dsm_postmaster_startup
> happens before the first PostmasterRandom call --- which is OK today but
> seems pretty fragile.  Still, redesigning PostmasterRandom's seeding
> technique is not something to do right before 9.6 release.  Let's revert
> the prior patch and do it as you have here:
>
>> struct timeval tv;
>> gettimeofday(&tv, NULL);
>> srandom(tv.tv_sec);
>> ...
>> dsm_control_handle = random();
>
> for the time being.
>

Isn't it better if we use the same technique in dsm_create() as well
which uses random() for handle?

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


[HACKERS] Re: [HACKERS] Re: [HACKERS] Re: [HACKERS] Re: [HACKERS] Windows service is not starting so there’s message in log: FATAL: "could not create shared memory segment “Global/PostgreSQL.851401618

2016-09-20 Thread Robert Haas
On Thu, May 26, 2016 at 7:44 PM, Michael Paquier
 wrote:
>> Thanks for reviewing the patch.  I have added the entry for this patch in
>> next CF (https://commitfest.postgresql.org/10/636/),  feel free to mark it
>> as Ready for committer if you think patch is good.
>
> Yeah, it is definitely better to register it. And I have switched the
> patch as ready for committer just now.

Committed and back-patched to 9.4, where DSM was introduced.

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


[HACKERS] Re: [HACKERS] Re: [HACKERS] Re: [HACKERS] Re: [HACKERS] Windows service is not starting so there’s message in log: FATAL: "could not create shared memory segment “Global/PostgreSQL.851401618

2016-05-26 Thread Michael Paquier
On Thu, May 26, 2016 at 3:01 AM, Amit Kapila  wrote:
> On Wed, May 25, 2016 at 9:44 PM, Michael Paquier 
> wrote:
>>
>> On Wed, May 25, 2016 at 12:11 AM, Amit Kapila 
>> wrote:
>> >
>> > Okay, attached patch just does that and I have verified that it allows
>> > to
>> > start multiple services in windows.  In off list discussion with Robert,
>> > he
>> > suggested not to complicate the patch by retrying for fixed number of
>> > times
>> > as there is no proof that ERROR_ACCESS_DENIED can occur due to any other
>> > reason in this code path.  This patch is based on Kyotaro san's patch
>> > posted
>> > upthread with just minor changes in comments and indentation.
>>
>> Thanks for catching Robert and getting confirmation on the matter. In
>> the same line of thoughts, I think as well that it is definitely not
>> worth complicating the retry logic in dsm.c, but you knew that already
>> per last week's discussion.
>>
>> Regarding the patch, this looks good to me.
>>
>
> Thanks for reviewing the patch.  I have added the entry for this patch in
> next CF (https://commitfest.postgresql.org/10/636/),  feel free to mark it
> as Ready for committer if you think patch is good.

Yeah, it is definitely better to register it. And I have switched the
patch as ready for committer just now.
-- 
Michael


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


[HACKERS] Re: [HACKERS] Re: [HACKERS] Re: [HACKERS] Re: [HACKERS] Windows service is not starting so there’s message in log: FATAL: "could not create shared memory segment “Global/PostgreSQL.851401618

2016-05-26 Thread Amit Kapila
On Wed, May 25, 2016 at 9:44 PM, Michael Paquier 
wrote:
>
> On Wed, May 25, 2016 at 12:11 AM, Amit Kapila 
wrote:
> >
> > Okay, attached patch just does that and I have verified that it allows
to
> > start multiple services in windows.  In off list discussion with
Robert, he
> > suggested not to complicate the patch by retrying for fixed number of
times
> > as there is no proof that ERROR_ACCESS_DENIED can occur due to any other
> > reason in this code path.  This patch is based on Kyotaro san's patch
posted
> > upthread with just minor changes in comments and indentation.
>
> Thanks for catching Robert and getting confirmation on the matter. In
> the same line of thoughts, I think as well that it is definitely not
> worth complicating the retry logic in dsm.c, but you knew that already
> per last week's discussion.
>
> Regarding the patch, this looks good to me.
>

Thanks for reviewing the patch.  I have added the entry for this patch in
next CF (https://commitfest.postgresql.org/10/636/),  feel free to mark it
as Ready for committer if you think patch is good.


With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com


[HACKERS] Re: [HACKERS] Re: [HACKERS] Re: [HACKERS] Re: [HACKERS] Windows service is not starting so there’s message in log: FATAL: "could not create shared memory segment “Global/PostgreSQL.851401618

2016-05-25 Thread Michael Paquier
On Wed, May 25, 2016 at 12:11 AM, Amit Kapila  wrote:
> On Tue, May 17, 2016 at 2:31 AM, Michael Paquier 
> wrote:
>>
>> On Tue, May 17, 2016 at 4:16 AM, Amit Kapila 
>> wrote:
>> > On Mon, May 16, 2016 at 9:45 AM, Michael Paquier
>> > 
>> > wrote:
>> >>
>> >> On Sun, May 15, 2016 at 3:34 PM, Amit Kapila 
>> >> wrote:
>> >> > Sounds sensible, but if we want to that route, shall we have some
>> >> > mechanism
>> >> > such that if retrying it for 10 times (10 is somewhat arbitrary, but
>> >> > we
>> >> > retry 10 times in PGSharedMemoryCreate, so may be there is some
>> >> > consistency)
>> >> > doesn't give us unique name and we are getting EACCES error, then
>> >> > just
>> >> > throw
>> >> > the error instead of more retries.  This is to ensure that if the API
>> >> > is
>> >> > returning EACCES due to reason other than duplicate handle, then we
>> >> > won't
>> >> > retry indefinitely.
>> >>
>> >> The logic in win32_shmem.c relies on the fact that a segment will be
>> >> recycled, and the retry is here because it may take time at OS level.
>> >> On top of that it relies on the segment names being unique across
>> >> systems. So it seems to me that it is not worth the complication to
>> >> duplicate that logic in the dsm implementation.
>> >
>> > If we don't do retry for fixed number of times, then how will we handle
>> > the
>> > case if EACCES is due to the reason other than duplicate handle?
>>
>> EACCES is a bit too low-level... I had in mind to check GetLastError
>> with only ERROR_ACCESS_DENIED, and retry only in this case, which is
>> the case where one postmaster is trying to access the segment of
>> another.
>>
>
> Okay, attached patch just does that and I have verified that it allows to
> start multiple services in windows.  In off list discussion with Robert, he
> suggested not to complicate the patch by retrying for fixed number of times
> as there is no proof that ERROR_ACCESS_DENIED can occur due to any other
> reason in this code path.  This patch is based on Kyotaro san's patch posted
> upthread with just minor changes in comments and indentation.

Thanks for catching Robert and getting confirmation on the matter. In
the same line of thoughts, I think as well that it is definitely not
worth complicating the retry logic in dsm.c, but you knew that already
per last week's discussion.

Regarding the patch, this looks good to me.
-- 
Michael


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


[HACKERS] Re: [HACKERS] Re: [HACKERS] Re: [HACKERS] Re: [HACKERS] Windows service is not starting so there’s message in log: FATAL: "could not create shared memory segment “Global/PostgreSQL.851401618

2016-05-25 Thread Amit Kapila
On Tue, May 17, 2016 at 2:31 AM, Michael Paquier 
wrote:
>
> On Tue, May 17, 2016 at 4:16 AM, Amit Kapila 
wrote:
> > On Mon, May 16, 2016 at 9:45 AM, Michael Paquier <
michael.paqu...@gmail.com>
> > wrote:
> >>
> >> On Sun, May 15, 2016 at 3:34 PM, Amit Kapila 
> >> wrote:
> >> > Sounds sensible, but if we want to that route, shall we have some
> >> > mechanism
> >> > such that if retrying it for 10 times (10 is somewhat arbitrary, but
we
> >> > retry 10 times in PGSharedMemoryCreate, so may be there is some
> >> > consistency)
> >> > doesn't give us unique name and we are getting EACCES error, then
just
> >> > throw
> >> > the error instead of more retries.  This is to ensure that if the
API is
> >> > returning EACCES due to reason other than duplicate handle, then we
> >> > won't
> >> > retry indefinitely.
> >>
> >> The logic in win32_shmem.c relies on the fact that a segment will be
> >> recycled, and the retry is here because it may take time at OS level.
> >> On top of that it relies on the segment names being unique across
> >> systems. So it seems to me that it is not worth the complication to
> >> duplicate that logic in the dsm implementation.
> >
> > If we don't do retry for fixed number of times, then how will we handle
the
> > case if EACCES is due to the reason other than duplicate handle?
>
> EACCES is a bit too low-level... I had in mind to check GetLastError
> with only ERROR_ACCESS_DENIED, and retry only in this case, which is
> the case where one postmaster is trying to access the segment of
> another.
>

Okay, attached patch just does that and I have verified that it allows to
start multiple services in windows.  In off list discussion with Robert, he
suggested not to complicate the patch by retrying for fixed number of times
as there is no proof that ERROR_ACCESS_DENIED can occur due to any other
reason in this code path.  This patch is based on Kyotaro san's patch
posted upthread with just minor changes in comments and indentation.


With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com


dsm_win_segment_access_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


[HACKERS] Re: [HACKERS] Re: [HACKERS] Re: [HACKERS] Re: [HACKERS] Windows service is not starting so there’s message in log: FATAL: "could not create shared memory segment “Global/PostgreSQL.851401618

2016-05-16 Thread Michael Paquier
On Tue, May 17, 2016 at 6:01 AM, Michael Paquier
 wrote:
> EACCES is a bit too low-level... I had in mind to check GetLastError
> with only ERROR_ACCESS_DENIED, and retry only in this case, which is
> the case where one postmaster is trying to access the segment of
> another.

s/low/high/.
-- 
Michael


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


[HACKERS] Re: [HACKERS] Re: [HACKERS] Re: [HACKERS] Re: [HACKERS] Windows service is not starting so there’s message in log: FATAL: "could not create shared memory segment “Global/PostgreSQL.851401618

2016-05-16 Thread Michael Paquier
On Tue, May 17, 2016 at 4:16 AM, Amit Kapila  wrote:
> On Mon, May 16, 2016 at 9:45 AM, Michael Paquier 
> wrote:
>>
>> On Sun, May 15, 2016 at 3:34 PM, Amit Kapila 
>> wrote:
>> > Sounds sensible, but if we want to that route, shall we have some
>> > mechanism
>> > such that if retrying it for 10 times (10 is somewhat arbitrary, but we
>> > retry 10 times in PGSharedMemoryCreate, so may be there is some
>> > consistency)
>> > doesn't give us unique name and we are getting EACCES error, then just
>> > throw
>> > the error instead of more retries.  This is to ensure that if the API is
>> > returning EACCES due to reason other than duplicate handle, then we
>> > won't
>> > retry indefinitely.
>>
>> The logic in win32_shmem.c relies on the fact that a segment will be
>> recycled, and the retry is here because it may take time at OS level.
>> On top of that it relies on the segment names being unique across
>> systems. So it seems to me that it is not worth the complication to
>> duplicate that logic in the dsm implementation.
>
> If we don't do retry for fixed number of times, then how will we handle the
> case if EACCES is due to the reason other than duplicate handle?

EACCES is a bit too low-level... I had in mind to check GetLastError
with only ERROR_ACCESS_DENIED, and retry only in this case, which is
the case where one postmaster is trying to access the segment of
another.
-- 
Michael


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


[HACKERS] Re: [HACKERS] Re: [HACKERS] Re: [HACKERS] Re: [HACKERS] Windows service is not starting so there’s message in log: FATAL: "could not create shared memory segment “Global/PostgreSQL.851401618

2016-05-16 Thread Amit Kapila
On Mon, May 16, 2016 at 9:45 AM, Michael Paquier 
wrote:
>
> On Sun, May 15, 2016 at 3:34 PM, Amit Kapila 
wrote:
> > Sounds sensible, but if we want to that route, shall we have some
mechanism
> > such that if retrying it for 10 times (10 is somewhat arbitrary, but we
> > retry 10 times in PGSharedMemoryCreate, so may be there is some
consistency)
> > doesn't give us unique name and we are getting EACCES error, then just
throw
> > the error instead of more retries.  This is to ensure that if the API is
> > returning EACCES due to reason other than duplicate handle, then we
won't
> > retry indefinitely.
>
> The logic in win32_shmem.c relies on the fact that a segment will be
> recycled, and the retry is here because it may take time at OS level.
> On top of that it relies on the segment names being unique across
> systems. So it seems to me that it is not worth the complication to
> duplicate that logic in the dsm implementation.

If we don't do retry for fixed number of times, then how will we handle the
case if EACCES is due to the reason other than duplicate handle?

With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com


[HACKERS] Re: [HACKERS] Re: [HACKERS] Re: [HACKERS] Re: [HACKERS] Windows service is not starting so there’s message in log: FATAL: "could not create shared memory segment “Global/PostgreSQL.851401618

2016-05-15 Thread Michael Paquier
On Sun, May 15, 2016 at 3:34 PM, Amit Kapila  wrote:
> On Sat, May 14, 2016 at 7:33 PM, Robert Haas  wrote:
>>
>> On Tue, Mar 22, 2016 at 12:56 AM, Amit Kapila 
>> wrote:
>> >> >> Yes, same random number generation is not the problem. In windows
>> >> >> apart
>> >> >> from EEXIST error, EACCES also needs to be validated and returned
>> >> >> for
>> >> >> new random number generation, instead of throwing an error.
>> >> >
>> >> > Doing the same handling for EACCES doesn't seem to be sane because if
>> >> > EACCES
>> >> > came for reason other than duplicate dsm name, then we want to report
>> >> > the
>> >> > error instead of trying to regenerate the name.  I think here fix
>> >> > should
>> >> > be
>> >> > to append data_dir path as we do for main shared memory.
>> >>
>> >> Yes, EACCES may be possible other than duplicate dsm name.
>> >
>> > So as far as I can see there are two ways to resolve this issue, one is
>> > to
>> > retry generation of dsm name if CreateFileMapping returns EACCES and
>> > second
>> > is to append data_dir name to dsm name as the same is done for main
>> > shared
>> > memory, that will avoid the error to occur.  First approach has minor
>> > flaw
>> > that if CreateFileMapping returns EACCES due to reason other then
>> > duplicate
>> > dsm name which I am not sure is possible to identify, then we should
>> > report
>> > error instead try to regenerate the name
>> >
>> > Robert and or others, can you share your opinion on what is the best way
>> > to
>> > proceed for this issue.
>>
>> I think we should retry on EACCES.  Possibly we should do other things
>> too, but at least that.  It completely misses the point of retrying on
>> EEXIST if we don't retry on other error codes that can also be
>> generated when the segment already exists.
>>

Well, if we don't care about segment uniqueness more than that... I
guess I will just throw the white flag. By retrying with a new segment
name at each loop, there is no risk to retry infinitely and remain
stuck, so let's just use something like
1444921511.3661.13.ca...@postgrespro.ru as a fix and call that a deal
(with a fatter comment). CreateFileMapping would return a handle only
with ERROR_ALREADY_EXISTS per the docs.

> Sounds sensible, but if we want to that route, shall we have some mechanism
> such that if retrying it for 10 times (10 is somewhat arbitrary, but we
> retry 10 times in PGSharedMemoryCreate, so may be there is some consistency)
> doesn't give us unique name and we are getting EACCES error, then just throw
> the error instead of more retries.  This is to ensure that if the API is
> returning EACCES due to reason other than duplicate handle, then we won't
> retry indefinitely.

The logic in win32_shmem.c relies on the fact that a segment will be
recycled, and the retry is here because it may take time at OS level.
On top of that it relies on the segment names being unique across
systems. So it seems to me that it is not worth the complication to
duplicate that logic in the dsm implementation.
-- 
Michael


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


[HACKERS] Re: [HACKERS] Re: [HACKERS] Re: [HACKERS] Re: [HACKERS] Windows service is not starting so there’s message in log: FATAL: "could not create shared memory segment “Global/PostgreSQL.851401618

2016-05-14 Thread Robert Haas
On Mon, May 9, 2016 at 3:17 AM, Michael Paquier
 wrote:
> On Tue, Mar 22, 2016 at 1:56 PM, Amit Kapila  wrote:
>> So as far as I can see there are two ways to resolve this issue, one is to
>> retry generation of dsm name if CreateFileMapping returns EACCES and second
>> is to append data_dir name to dsm name as the same is done for main shared
>> memory, that will avoid the error to occur.  First approach has minor flaw
>> that if CreateFileMapping returns EACCES due to reason other then duplicate
>> dsm name which I am not sure is possible to identify, then we should report
>> error instead try to regenerate the name
>>
>> Robert and or others, can you share your opinion on what is the best way to
>> proceed for this issue.
>
> For my 2c here, the approach using GetSharedMemName to identify the
> origin of a dynamic shared memory segment looks more solid in terms of
> robustness and collision handling. Retrying a segment is never going
> to be completely water-proof.

Why not?  I mean, there are ~2^32 possible segment handles, and not
all that many of them can be in use.

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


[HACKERS] Re: [HACKERS] Re: [HACKERS] Re: [HACKERS] Re: [HACKERS] Windows service is not starting so there’s message in log: FATAL: "could not create shared memory segment “Global/PostgreSQL.851401618

2016-05-09 Thread Michael Paquier
On Mon, May 9, 2016 at 4:17 PM, Michael Paquier
 wrote:
> On Tue, Mar 22, 2016 at 1:56 PM, Amit Kapila  wrote:
>> So as far as I can see there are two ways to resolve this issue, one is to
>> retry generation of dsm name if CreateFileMapping returns EACCES and second
>> is to append data_dir name to dsm name as the same is done for main shared
>> memory, that will avoid the error to occur.  First approach has minor flaw
>> that if CreateFileMapping returns EACCES due to reason other then duplicate
>> dsm name which I am not sure is possible to identify, then we should report
>> error instead try to regenerate the name
>>
>> Robert and or others, can you share your opinion on what is the best way to
>> proceed for this issue.
>
> For my 2c here, the approach using GetSharedMemName to identify the
> origin of a dynamic shared memory segment looks more solid in terms of
> robustness and collision handling. Retrying a segment is never going
> to be completely water-proof.

So, I have been hacking that a bit more and finished with the
attached, which seem to address the issue here. Some of the code paths
of dsm_impl.c are done in such a way that we can fail a dsm allocation
and still continue to move on. I have taken that into account by using
palloc_extended with NO_OOM. palloc instead of malloc is a good fit
anyway to prevent leaks in error code paths.

Thoughts?
-- 
Michael
diff --git a/src/backend/storage/ipc/dsm_impl.c b/src/backend/storage/ipc/dsm_impl.c
index 173b982..060d0cb 100644
--- a/src/backend/storage/ipc/dsm_impl.c
+++ b/src/backend/storage/ipc/dsm_impl.c
@@ -62,6 +62,7 @@
 #include 
 #endif
 
+#include "miscadmin.h"
 #include "portability/mem.h"
 #include "storage/dsm_impl.h"
 #include "storage/fd.h"
@@ -80,6 +81,7 @@ static bool dsm_impl_sysv(dsm_op op, dsm_handle handle, Size request_size,
 			  Size *mapped_size, int elevel);
 #endif
 #ifdef USE_DSM_WINDOWS
+static char *get_segment_name(dsm_handle handle);
 static bool dsm_impl_windows(dsm_op op, dsm_handle handle, Size request_size,
  void **impl_private, void **mapped_address,
  Size *mapped_size, int elevel);
@@ -590,6 +592,36 @@ dsm_impl_sysv(dsm_op op, dsm_handle handle, Size request_size,
 
 #ifdef USE_DSM_WINDOWS
 /*
+ * Generate shared memory segment name, using the data directory to generate
+ * a unique identifier. This segment is stored in Global\ namespace to allow
+ * any other process to have an access to it.
+ */
+static char *
+get_segment_name(dsm_handle handle)
+{
+	char	   *res;
+
+	/* let enough place for prefix and handle */
+	res = palloc_extended(strlen(DataDir) + 64, MCXT_ALLOC_NO_OOM);
+
+	if (res == NULL)
+		return NULL;
+
+	/*
+	 * Storing the shared memory segment in the Global\ namespace, can allow
+	 * any process running in any session to access that file mapping object
+	 * provided that the caller has the required access rights. But to avoid
+	 * issues faced in main shared memory, we are using the naming convention
+	 * similar to main shared memory. We can change here once issue mentioned
+	 * in GetSharedMemName is resolved.
+	 */
+	snprintf(res, strlen(DataDir) + 64, "%s.%s.%u",
+			 SEGMENT_NAME_PREFIX, DataDir, handle);
+
+	return res;
+}
+
+/*
  * Operating system primitives to support Windows shared memory.
  *
  * Windows shared memory implementation is done using file mapping
@@ -609,7 +641,7 @@ dsm_impl_windows(dsm_op op, dsm_handle handle, Size request_size,
 {
 	char	   *address;
 	HANDLE		hmap;
-	char		name[64];
+	char	   *name;
 	MEMORY_BASIC_INFORMATION info;
 
 	/* Resize is not supported for Windows shared memory. */
@@ -623,15 +655,13 @@ dsm_impl_windows(dsm_op op, dsm_handle handle, Size request_size,
 	if (op == DSM_OP_ATTACH && *mapped_address != NULL)
 		return true;
 
-	/*
-	 * Storing the shared memory segment in the Global\ namespace, can allow
-	 * any process running in any session to access that file mapping object
-	 * provided that the caller has the required access rights. But to avoid
-	 * issues faced in main shared memory, we are using the naming convention
-	 * similar to main shared memory. We can change here once issue mentioned
-	 * in GetSharedMemName is resolved.
-	 */
-	snprintf(name, 64, "%s.%u", SEGMENT_NAME_PREFIX, handle);
+	name = get_segment_name(handle);
+
+	if (name == NULL)
+	{
+		elog(elevel, "could not allocate memory for shared memory segment name");
+		return false;
+	}
 
 	/*
 	 * Handle teardown cases.  Since Windows automatically destroys the object
@@ -647,6 +677,7 @@ dsm_impl_windows(dsm_op op, dsm_handle handle, Size request_size,
 	(errcode_for_dynamic_shared_memory(),
    errmsg("could not unmap shared memory segment \"%s\": %m",
 		  name)));
+			pfree(name);
 			return false;
 		}
 		if (*impl_private != NULL
@@ -657,12 +688,14 @@ dsm_impl_windows(dsm_op op, dsm_handle handle, Size request_size,
 	(errcode_for_dynamic_shared_memory(),
   errmsg("could not remove shared memory segment \"%s\": %m",
 		 name)));
+			

[HACKERS] Re: [HACKERS] Re: [HACKERS] Re: [HACKERS] Re: [HACKERS] Windows service is not starting so there’s message in log: FATAL: "could not create shared memory segment “Global/PostgreSQL.851401618

2016-05-09 Thread Michael Paquier
On Tue, Mar 22, 2016 at 1:56 PM, Amit Kapila  wrote:
> So as far as I can see there are two ways to resolve this issue, one is to
> retry generation of dsm name if CreateFileMapping returns EACCES and second
> is to append data_dir name to dsm name as the same is done for main shared
> memory, that will avoid the error to occur.  First approach has minor flaw
> that if CreateFileMapping returns EACCES due to reason other then duplicate
> dsm name which I am not sure is possible to identify, then we should report
> error instead try to regenerate the name
>
> Robert and or others, can you share your opinion on what is the best way to
> proceed for this issue.

For my 2c here, the approach using GetSharedMemName to identify the
origin of a dynamic shared memory segment looks more solid in terms of
robustness and collision handling. Retrying a segment is never going
to be completely water-proof.
-- 
Michael


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


[HACKERS] Re: [HACKERS] Re: [HACKERS] Re: [HACKERS] Re: [HACKERS] Windows service is not starting so there’s message in log: FATAL: "could not create shared memory segment “Global/PostgreSQL.851401618

2016-03-21 Thread Craig Ringer
On 21 March 2016 at 20:46, Haribabu Kommi  wrote:


>
> No. Not as local service. The user should be the new standard user
> that is created
> in the system.
>

Which was done how, exactly?

Commands run? Steps taken?

PostgreSQL drops privileges once it starts, so it's actually pretty OK to
run it as an admin user, NetworkService, etc.

Otherwise you should really make a service user, not a regular user
account. Don't allow the account to log in interactively, do allow it to
log in as a service. Don't make it a domain account unless you need domain
integration for SSPI etc.

The best option on newer Windows should be to use a managed service account
(https://technet.microsoft.com/en-us/library/ff641731(v=ws.10).aspx) or
virtual account. Eventually the installer should switch to doing that
automatically instead of using NetworkService.


> >> 5. Now try to start the services, the second service fails with the
> >> error message.
> >> 6. Error details can be found out in Event log viewer.
>

Can you get a Process Monitor trace of startup and check exactly where it's
getting access denied, doing what?

You may have to dig through a *lot* of output to find it.

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