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-09 02:21:36 |
Message-ID: | CAEepm=2RqzvRyEXWxq8B9zR8p45tt-xsvA4S8ZNKNwpiUzMVKA@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Tue, Oct 9, 2018 at 1:53 PM Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Thomas Munro <thomas(dot)munro(at)enterprisedb(dot)com> writes:
> > On Mon, Oct 8, 2018 at 1:17 AM Thomas Munro
> > <thomas(dot)munro(at)enterprisedb(dot)com> wrote:
> >> That's because the bgworker startup path doesn't contain a call to
> >> srandom(...distinguishing stuff...), unlike BackendRun(). I suppose
> >> do_start_bgworker() could gain a similar call... or perhaps that call
> >> should be moved into InitPostmasterChild(). If we put it in there
> >> right after MyStartTime is assigned a new value, we could use the same
> >> incantation that PostmasterMain() uses.
>
> > Maybe something like this?
>
> I think the bit with
>
> #ifndef HAVE_STRONG_RANDOM
> random_seed = 0;
> random_start_time.tv_usec = 0;
> #endif
>
> should also be done in every postmaster child, no? If we want to hide the
> postmaster's state from child processes, we ought to hide it from all.
Ok, here is a sketch patch with a new function InitRandomSeeds() to do
that. It is called from PostmasterMain(), InitPostmasterChild() and
InitStandaloneProcess() for consistency.
It seems a bit strange to me that internal infrastructure shares a
random number generator with SQL-callable functions random() and
setseed(), though I'm not saying it's harmful.
While wondering if something like this should be back-patched, I
noticed that SQL random() is declared as parallel-restricted, which is
good: it means we aren't exposing a random() function that generates
the same values in every parallel worker. So I think this is probably
just a minor nuisance and should probably only be pushed to master, or
at most to 11 (since Parallel Hash likes to create DSM segments in
workers), unless someone can think of a more serious way this can hurt
you.
(Tangentially: I suppose it might be useful to have a different SQL
random number function that is parallel safe, that isn't associated
with a user-controllable seed, and whose seed is different in each
backend.)
--
Thomas Munro
http://www.enterprisedb.com
Attachment | Content-Type | Size |
---|---|---|
0001-Make-sure-we-initialize-random-seeds-per-backend.patch | application/octet-stream | 4.6 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Michael Paquier | 2018-10-09 02:26:42 | Re: Function for listing archive_status directory |
Previous Message | Iwata, Aya | 2018-10-09 02:14:52 | RE: Function for listing archive_status directory |