Re: Retain dynamic shared memory segments for postmaster lifetime

From: Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>
To: Kyotaro HORIGUCHI <horiguchi(dot)kyotaro(at)lab(dot)ntt(dot)co(dot)jp>
Cc: pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Retain dynamic shared memory segments for postmaster lifetime
Date: 2014-02-12 09:35:41
Message-ID: CAA4eK1+MNSBvou97wMYGk2z0uNw7i5WHXFvX=NG2u9tkORU8AQ@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Wed, Feb 12, 2014 at 2:02 PM, Kyotaro HORIGUCHI
<horiguchi(dot)kyotaro(at)lab(dot)ntt(dot)co(dot)jp> wrote:
> Hello,
>
>> Please find new version of patch attached with this mail containing
>> above changes.
>
> This patch applies cleanly on current HEAD and build completed
> successfully on both Windows and Linux. (but master needed to be
> rewinded to some time ago for some compile errors.)
>
> This works correctly as before.
>
> Finally before send to commiters, would you mind changing the
> name of the segment "Global/PostgreSQL.%u" as
> 'Global/PostgreSQL.dsm.%u" or something mentioned in the last my
> email?

Actually in that case, to maintain consistency we need to change even in
function dsm_impl_posix() which uses segment name as "/PostgreSQL.%u".
For windows, we have added "Global" to "/PostgreSQL.%u", as it is
mandate by windows spec.
To be honest, I see no harm in changing the name as per your suggestion,
as it can improve segment naming for dynamic shared memory segments,
however there is no clear problem with current name as well, so I don't
want to change in places this patch has no relation.

I think best thing to do here is to put it as Notes To Committer, something
like:
Some suggestions for Committer to consider:
"Change the name of dsm segments from .. to .."

In general, what I see is that they consider all discussion in thread, but
putting some special notes like above will reduce the chance of getting
overlooked by them. I have done as a reviewer previously and it worked
well.

> However, it is a bit different thing from this patch so
> I have no intention to compel to do the changing.

Thanks to you for understanding my position.

Thanks for reviewing the patch so carefully, especially Windows part
which I think was bit tricky for you to setup.

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

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Andres Freund 2014-02-12 10:14:56 Re: Patch: show xid and xmin in pg_stat_activity and pg_stat_replication
Previous Message Tomonari Katsumata 2014-02-12 09:31:23 Re: [BUG] Archive recovery failure on 9.3+.