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:
>> http://www.postgresql.org/message-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.
>
> 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
