This is wrong, current code does well for this case. I should broke the code during investigating the problem.
> > 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. > > The patch attached will fix *both of* the problems. So, the patch fixes only the "Permission denied" case. regards, At Fri, 16 Oct 2015 14:37:47 +0900 (Tokyo Standard Time), Kyotaro HORIGUCHI <horiguchi.kyot...@lab.ntt.co.jp> wrote in <20151016.143747.121283630.horiguchi.kyot...@lab.ntt.co.jp> > > > 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 <amit.kapil...@gmail.com> > > 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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
