On Thu, Feb 6, 2014 at 3:42 PM, Kyotaro HORIGUCHI
<horiguchi.kyotaro@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
