Re: [HACKERS] Re: [COMMITTERS] pgsql: Perform only one ReadControlFile() during startup.

2017-09-19 Thread Andres Freund
On 2017-09-19 13:15:28 -0400, Tom Lane wrote: > Andres Freund writes: > > On 2017-09-19 13:00:33 -0400, Robert Haas wrote: > >> You mean, in the postmaster? > > > Yes. We try to avoid touch shmem there, but it's not like we're > > succeeding fully. See e.g. the

Re: [HACKERS] Re: [COMMITTERS] pgsql: Perform only one ReadControlFile() during startup.

2017-09-19 Thread Tom Lane
Andres Freund writes: > On 2017-09-19 13:00:33 -0400, Robert Haas wrote: >> You mean, in the postmaster? > Yes. We try to avoid touch shmem there, but it's not like we're > succeeding fully. See e.g. the pgstat_get_crashed_backend_activity() > calls (which do rely on shmem

Re: [HACKERS] Re: [COMMITTERS] pgsql: Perform only one ReadControlFile() during startup.

2017-09-19 Thread Andres Freund
On 2017-09-19 13:00:33 -0400, Robert Haas wrote: > On Tue, Sep 19, 2017 at 12:51 PM, Andres Freund wrote: > > That'd not be that a crazy amount of > > shared memory that'd need to be touched in shared memory, ... > > You mean, in the postmaster? Yes. We try to avoid touch

Re: [HACKERS] Re: [COMMITTERS] pgsql: Perform only one ReadControlFile() during startup.

2017-09-19 Thread Robert Haas
On Tue, Sep 19, 2017 at 12:51 PM, Andres Freund wrote: > That'd not be that a crazy amount of > shared memory that'd need to be touched in shared memory, ... You mean, in the postmaster? -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL

Re: [HACKERS] Re: [COMMITTERS] pgsql: Perform only one ReadControlFile() during startup.

2017-09-19 Thread Andres Freund
On 2017-09-19 12:24:00 -0400, Tom Lane wrote: > Andres Freund writes: > > Unfortunately the backends themselves also react with inaccurate error > > messages to things like immediate shutdowns... > > Yeah, those signals are kind of overloaded these days. Not sure if >

Re: [HACKERS] Re: [COMMITTERS] pgsql: Perform only one ReadControlFile() during startup.

2017-09-19 Thread Tom Lane
Andres Freund writes: > Unfortunately the backends themselves also react with inaccurate error > messages to things like immediate shutdowns... Yeah, those signals are kind of overloaded these days. Not sure if there's any good way to improve that.

Re: [HACKERS] Re: [COMMITTERS] pgsql: Perform only one ReadControlFile() during startup.

2017-09-19 Thread Andres Freund
On 2017-09-19 07:48:42 -0400, Robert Haas wrote: > Oh, I've not seen that. Mostly, what I think we should fix is the > fact that the libpq messages tend to report that the server crashed > even if it was an orderly shutdown. > > [rhaas ~]$ psql > psql (11devel) > Type "help" for help. > >

Re: [HACKERS] Re: [COMMITTERS] pgsql: Perform only one ReadControlFile() during startup.

2017-09-19 Thread Robert Haas
On Mon, Sep 18, 2017 at 2:04 PM, Andres Freund wrote: > On 2017-09-18 12:16:42 -0400, Robert Haas wrote: >> On Mon, Sep 18, 2017 at 6:32 AM, Andres Freund wrote: >> > One thing that I've noticed for a while, but that I was reminded of >> > again here. We

Re: [HACKERS] Re: [COMMITTERS] pgsql: Perform only one ReadControlFile() during startup.

2017-09-18 Thread Tom Lane
Andres Freund writes: > On 2017-09-18 12:16:42 -0400, Robert Haas wrote: >> I don't understand what you're talking about here. > I often see a backend crash, psql reacting to that crash by > reconnecting, successfully establish a new connection, just to be kicked > off by

Re: [HACKERS] Re: [COMMITTERS] pgsql: Perform only one ReadControlFile() during startup.

2017-09-18 Thread Andres Freund
On 2017-09-18 12:16:42 -0400, Robert Haas wrote: > On Mon, Sep 18, 2017 at 6:32 AM, Andres Freund wrote: > > One thing that I've noticed for a while, but that I was reminded of > > again here. We very frequently allow psql to reconnect in case of crash, > > just for postmaster

Re: [HACKERS] Re: [COMMITTERS] pgsql: Perform only one ReadControlFile() during startup.

2017-09-18 Thread Simon Riggs
On 16 September 2017 at 21:27, Andres Freund wrote: > On 2017-09-16 15:59:01 -0400, Tom Lane wrote: >> This does not seem like a problem that justifies a system-wide change >> that's much more delicate than you thought. +1 The change/reason ratio is too high. -- Simon

Re: [HACKERS] Re: [COMMITTERS] pgsql: Perform only one ReadControlFile() during startup.

2017-09-18 Thread Robert Haas
On Mon, Sep 18, 2017 at 6:32 AM, Andres Freund wrote: > One thing that I've noticed for a while, but that I was reminded of > again here. We very frequently allow psql to reconnect in case of crash, > just for postmaster to notice a child has gone and kill that session. I >

Re: [HACKERS] Re: [COMMITTERS] pgsql: Perform only one ReadControlFile() during startup.

2017-09-18 Thread Andres Freund
On 2017-09-17 01:07:52 -0700, Andres Freund wrote: > On 2017-09-16 13:27:05 -0700, Andres Freund wrote: > > > This does not seem like a problem that justifies a system-wide change > > > that's much more delicate than you thought. > > > > We need one more initialization call during crash-restart -

Re: [HACKERS] Re: [COMMITTERS] pgsql: Perform only one ReadControlFile() during startup.

2017-09-17 Thread Andres Freund
On 2017-09-16 13:27:05 -0700, Andres Freund wrote: > > This does not seem like a problem that justifies a system-wide change > > that's much more delicate than you thought. > > We need one more initialization call during crash-restart - that doesn't > seem particularly hard a fix. FWIW, attached

Re: [HACKERS] Re: [COMMITTERS] pgsql: Perform only one ReadControlFile() during startup.

2017-09-16 Thread Andres Freund
On 2017-09-16 15:59:01 -0400, Tom Lane wrote: > Andres Freund writes: > > On 2017-09-16 14:30:21 -0400, Tom Lane wrote: > >> I wonder whether we shouldn't just revert this patch altogether. > > > The problem is that the patch that makes the segment size configurable > > also

Re: [HACKERS] Re: [COMMITTERS] pgsql: Perform only one ReadControlFile() during startup.

2017-09-16 Thread Tom Lane
Andres Freund writes: > On 2017-09-16 14:30:21 -0400, Tom Lane wrote: >> I wonder whether we shouldn't just revert this patch altogether. > The problem is that the patch that makes the segment size configurable > also adds a bunch more ordering constraints due to the fact

Re: [HACKERS] Re: [COMMITTERS] pgsql: Perform only one ReadControlFile() during startup.

2017-09-16 Thread Andres Freund
On 2017-09-16 14:30:21 -0400, Tom Lane wrote: > Andres Freund writes: > > Looking into it. > > I wonder whether we shouldn't just revert this patch altogether. > Certainly, extra reads of pg_control are not a noticeable performance > problem. The problem is that the patch

Re: [HACKERS] Re: [COMMITTERS] pgsql: Perform only one ReadControlFile() during startup.

2017-09-16 Thread Tom Lane
Andres Freund writes: > Looking into it. I wonder whether we shouldn't just revert this patch altogether. Certainly, extra reads of pg_control are not a noticeable performance problem. I'm now quite worried about whether we aren't introducing hazards of using stale values

[HACKERS] Re: [COMMITTERS] pgsql: Perform only one ReadControlFile() during startup.

2017-09-16 Thread Andres Freund
Hi, On 2017-09-16 10:32:29 -0400, Tom Lane wrote: > It's dying at "pfree(localControlFile)". localControlFile seems to > be pointing at a region of memory that's entirely zeroes; certainly > the data that it just moved into shared memory is all zeroes. > It looks like someone didn't think hard