Re: DSM segment handle generation in background workers

From: Thomas Munro <thomas(dot)munro(at)enterprisedb(dot)com>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Pg Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: DSM segment handle generation in background workers
Date: 2018-10-19 01:01:44
Message-ID: CAEepm=0j8XfM1ww54XrPY1hySktkBzG_R3m_EsUS48bxjBQy=A@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Sat, Oct 13, 2018 at 11:45 PM Thomas Munro
<thomas(dot)munro(at)enterprisedb(dot)com> wrote:
> On Thu, Oct 11, 2018 at 5:51 PM Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> > The comment adjacent to the change in InitStandaloneProcess bothers me.
> > In particular, it points out that what BackendRun() is currently doing
> > creates more entropy in the process's random seed than what you have
> > here: MyStartTime is only good to the nearest second, whereas the code
> > you removed from BackendRun() factors in fractional seconds to whatever
> > the precision of GetCurrentTimestamp is. This does not seem like an
> > improvement.
>
> True.
>
> > I'm inclined to think we should refactor a bit more aggressively so
> > that, rather than dumbing backends down to the standard of other
> > processes, we make them all use the better method. A reasonable way
> > to approach this would be to invent a global variable MyStartTimestamp
> > beside MyStartTime, and initialize both of them in the places that
> > currently initialize the latter, using code like that in
> > BackendInitialize:
> >
> > /* save process start time */
> > port->SessionStartTime = GetCurrentTimestamp();
> > MyStartTime = timestamptz_to_time_t(port->SessionStartTime);
> >
> > and then this new InitRandomSeeds function could adopt BackendRun's
> > seed initialization method instead of the stupider way.
>
> Ok, done.
>
> With MyStartTimestamp in the picture, port->SessionStartTime is
> probably redundant... <looks around> and in fact the comment in struct
> Port says it shouldn't even be there. So... I removed it, and used
> the new MyStartTimestamp in the couple of places that wanted it.
> Thoughts?
>
> > Possibly it'd be sane to merge the MyStartTime* initializations
> > and the srandom resets into one function, not sure.
>
> +1
>
> The main difficulty was coming up with a name for the function that
> does those things. I went with InitProcessGlobals(). Better
> suggestions welcome.

Pushed to master only.

--
Thomas Munro
http://www.enterprisedb.com

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Amit Langote 2018-10-19 04:05:52 Re: partition tree inspection functions
Previous Message Thomas Munro 2018-10-19 00:25:58 Re: lowering pg_regress privileges on Windows