Hello,
> Currently we are using error level as ERROR for creating dsm during
> postmaster startup which is not right and rather we should use error
> level as LOG. Can you please try with the attached patch and see
> if the issue is fixed for you.
It seems somewhat strange. Looking into the create operation in
dms_impl_posix(), it should return false but not emit error when
the shared memory object already exists.
So, to make the windows version behave as the same,
dsm_impl_windows should return false if GetLastError() ==
ERROR_ALREADY_EXISTS regardless of hmap is valid. The current
behavior is wrong because two or more postmaster *share* the same
DSM segment instead of having their own segments.
On the other hand, in the case of GetLastError() ==
ERROR_ACCESS_DENIED, which occurs between postmasters exexuted as
service, it can reasonablly be assumed that the segment is
already created by someone else. I think this is no problem
because postgres processes don't need to access someone else's
segments.
Finally, the valid fix would be that make it return false if
GetLastError() == ERROR_ALREADY_EXISTS or ERROR_ACCESS_DENIED.
The patch attached will fix *both of* the problems.
regards,
At Fri, 16 Oct 2015 09:02:41 +0530, Amit Kapila <[email protected]> wrote
in <caa4ek1+pjtdouf-lc9y0ugfdmwhnaf4kwim1y3anq0wz1gw...@mail.gmail.com>
> > I think that function dsm_impl_windows() with EACCES error should not
> > do ereport() with FATAL level. It works, but it is likely to make an
> > infinite loop if the user will receive EACCES error.
> >
> >
> Currently we are using error level as ERROR for creating dsm during
> postmaster startup which is not right and rather we should use error
> level as LOG. Can you please try with the attached patch and see
> if the issue is fixed for you.
>
> Another some what related point is currently we are using random()
> function to ensure a unique name for dsm and it seems to me that
> it is always going to generate same number on first invocation (at least
> thats what happening on windows) due to which you are seeing the
> error. Another options could be to append current pid or data directory
> path as we are doing in win32_shmem.c. I think this could be an
> optimization which can be done in addition to the fix attached (we can
> do this as a separate patch as well, if we agreed to do anything).
--
Kyotaro Horiguchi
NTT Open Source Software Center
diff --git a/src/backend/storage/ipc/dsm_impl.c b/src/backend/storage/ipc/dsm_impl.c
index 921f029..1070812 100644
--- a/src/backend/storage/ipc/dsm_impl.c
+++ b/src/backend/storage/ipc/dsm_impl.c
@@ -671,6 +671,7 @@ dsm_impl_windows(dsm_op op, dsm_handle handle, Size request_size,
{
DWORD size_high;
DWORD size_low;
+ DWORD errcode;
/* Shifts >= the width of the type are undefined. */
#ifdef _WIN64
@@ -686,9 +687,21 @@ dsm_impl_windows(dsm_op op, dsm_handle handle, Size request_size,
size_high, /* Upper 32 bits of size */
size_low, /* Lower 32 bits of size */
name);
+
+ /*
+ * If the mapping already exists, CreateFileMapping returns a valid
+ * handle but sets ERROR_ALREADY_EXISTS. Return false for the case.
+ * CreateFileMapping may return ERROR_ACCESS_DENIED for the cases when
+ * running as a service. It also tells that the mapping is already
+ * created by someone else, so return false.
+ */
+ errcode = GetLastError();
+ if (errcode == ERROR_ALREADY_EXISTS || errcode == ERROR_ACCESS_DENIED)
+ return false;
+
if (!hmap)
{
- _dosmaperr(GetLastError());
+ _dosmaperr(errcode);
ereport(elevel,
(errcode_for_dynamic_shared_memory(),
errmsg("could not create shared memory segment \"%s\": %m",
--
Sent via pgsql-hackers mailing list ([email protected])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
