From: Tom Lane <tgl@sss.pgh.pa.us>
To: Thomas Munro <thomas.munro@enterprisedb.com>
cc: Pg Hackers <pgsql-hackers@postgresql.org>
Subject: Re: DSM segment handle generation in background workers
In-reply-to: 
 <CAEepm=1xxETRCZexNSxkABpNg-65U6Z2pxynJ8V0vURqob+Rqw@mail.gmail.com>
References: 
 <CAEepm=2eJj_6=B+2tEpGu2nf1BjthCf9nXXUouYvJJ4C5WSwhg@mail.gmail.com>
 <CAEepm=1oRxe3n3a6bq23-2MQTSWagHTghPbpGJ1YcZEyZ_KWQA@mail.gmail.com>
 <17043.1539046409@sss.pgh.pa.us>
 <CAEepm=2RqzvRyEXWxq8B9zR8p45tt-xsvA4S8ZNKNwpiUzMVKA@mail.gmail.com>
 <CAEepm=1xxETRCZexNSxkABpNg-65U6Z2pxynJ8V0vURqob+Rqw@mail.gmail.com>
Comments: In-reply-to Thomas Munro <thomas.munro@enterprisedb.com>
	message dated "Thu, 11 Oct 2018 16:18:40 +1300"
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-ID: <9878.1539233472.1@sss.pgh.pa.us>
Date: Thu, 11 Oct 2018 00:51:12 -0400
Message-ID: <9879.1539233472@sss.pgh.pa.us>
Precedence: bulk

Thomas Munro <thomas.munro@enterprisedb.com> writes:
>> Ok, here is a sketch patch with a new function InitRandomSeeds() to do
>> that.  It is called from PostmasterMain(), InitPostmasterChild() and
>> InitStandaloneProcess() for consistency.

> Here's a version I propose to push to master and 11 tomorrow, if there
> are no objections.

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.

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.

Possibly it'd be sane to merge the MyStartTime* initializations
and the srandom resets into one function, not sure.

			regards, tom lane