Re: [HACKERS] Latch for the WAL writer - further reducing idle wake-ups.

2012-05-08 Thread Tom Lane
Peter Geoghegan pe...@2ndquadrant.com writes: Attached patch removes the questionable SetLatch() call, under the assumption that it's okay if the WALWriter, having entered hibernation due to sustained inactivity (10 seconds) subsequently takes up to 5 seconds (2.5 on average) to notice that it

Re: [HACKERS] Latch for the WAL writer - further reducing idle wake-ups.

2012-05-08 Thread Peter Geoghegan
On 8 May 2012 22:35, Tom Lane t...@sss.pgh.pa.us wrote: Now that I've actually read the patch, rather than just responding to your description of it, I find myself entirely unhappy with the proposed changes in the walwriter's sleep logic.  You have introduced race conditions (it is NOT okay to

Re: [HACKERS] Latch for the WAL writer - further reducing idle wake-ups.

2012-05-08 Thread Tom Lane
Peter Geoghegan pe...@2ndquadrant.com writes: On 8 May 2012 22:35, Tom Lane t...@sss.pgh.pa.us wrote: Now that I've actually read the patch, rather than just responding to your description of it, I find myself entirely unhappy with the proposed changes in the walwriter's sleep logic.  You have

Re: [HACKERS] Latch for the WAL writer - further reducing idle wake-ups.

2012-05-08 Thread Peter Geoghegan
On 9 May 2012 00:21, Peter Geoghegan pe...@2ndquadrant.com wrote: Yes, there is some checking of flags before the potential ResetLatch() call, which may be acted on. The code there is almost identical to that of the extant bgwriter code. I was under the impression that this did not amount to a

Re: [HACKERS] Latch for the WAL writer - further reducing idle wake-ups.

2012-05-08 Thread Tom Lane
I've applied the walwriter/checkpointer patch, with the mentioned re-simplification of the logic. While measuring that, I noticed that the stats collector was now the biggest repeated-wakeup culprit, so I took the time to latch-ify it as well. AFAICS we no longer have anything that wakes up

Re: [HACKERS] Latch for the WAL writer - further reducing idle wake-ups.

2012-05-08 Thread Tom Lane
On further reflection I've realized that there's a really unpleasant consequence of the walwriter change as-committed: it breaks the former guarantee that async commits would reach disk within at most 3 times the WalWriterDelay setting. They will still get written within at most 3 walwriter

Re: [HACKERS] Latch for the WAL writer - further reducing idle wake-ups.

2012-05-08 Thread Tom Lane
Peter Geoghegan pe...@2ndquadrant.com writes: On 9 May 2012 00:21, Peter Geoghegan pe...@2ndquadrant.com wrote: Yes, there is some checking of flags before the potential ResetLatch() call, which may be acted on. The code there is almost identical to that of the extant bgwriter code. I was

Re: [HACKERS] Latch for the WAL writer - further reducing idle wake-ups.

2012-05-07 Thread Tom Lane
Peter Geoghegan pe...@2ndquadrant.com writes: This latest revision also covers the checkpointer. The code for that is far simpler than that for the WAL Writer, so it doesn't particularly feel like I'm pushing my luck by slipping that into something to be slipped in. Well ... maybe, or maybe

Re: [HACKERS] Latch for the WAL writer - further reducing idle wake-ups.

2012-05-07 Thread Simon Riggs
On 7 May 2012 18:09, Tom Lane t...@sss.pgh.pa.us wrote: Peter Geoghegan pe...@2ndquadrant.com writes: This latest revision also covers the checkpointer. The code for that is far simpler than that for the WAL Writer, so it doesn't particularly feel like I'm pushing my luck by slipping that into

Re: [HACKERS] Latch for the WAL writer - further reducing idle wake-ups.

2012-05-07 Thread Tom Lane
Simon Riggs si...@2ndquadrant.com writes: On 7 May 2012 18:09, Tom Lane t...@sss.pgh.pa.us wrote: I also notice that the separate-checkpointer patch failed to rename assorted things like BgWriterCommLock, BgWriterRequest, BgWriterShmemStruct, which are all 100% inappropriately named now. And

Re: [HACKERS] Latch for the WAL writer - further reducing idle wake-ups.

2012-05-07 Thread Simon Riggs
On 7 May 2012 19:44, Tom Lane t...@sss.pgh.pa.us wrote: Simon Riggs si...@2ndquadrant.com writes: On 7 May 2012 18:09, Tom Lane t...@sss.pgh.pa.us wrote: I also notice that the separate-checkpointer patch failed to rename assorted things like BgWriterCommLock, BgWriterRequest,

Re: [HACKERS] Latch for the WAL writer - further reducing idle wake-ups.

2012-05-07 Thread Tom Lane
Simon Riggs si...@2ndquadrant.com writes: It also leaves the situation that we have a catalog view called pg_stat_bgwriter that would be accessing checkpointer things. That's really the thorny one that I wasn't sure how to handle. Good example of why we shouldn't expose internals too much.

Re: [HACKERS] Latch for the WAL writer - further reducing idle wake-ups.

2012-05-07 Thread Simon Riggs
On 7 May 2012 20:06, Tom Lane t...@sss.pgh.pa.us wrote: Simon Riggs si...@2ndquadrant.com writes: It also leaves the situation that we have a catalog view called pg_stat_bgwriter that would be accessing checkpointer things. That's really the thorny one that I wasn't sure how to handle. Good

Re: [HACKERS] Latch for the WAL writer - further reducing idle wake-ups.

2012-05-03 Thread Heikki Linnakangas
On 03.05.2012 03:41, Robert Haas wrote: On Wed, May 2, 2012 at 7:21 PM, Tom Lanet...@sss.pgh.pa.us wrote: Adding any contention at all to XLogInsert doesn't seem like a smart idea, even if you failed to measure any problem in the specific tests you made. I wonder whether we could not improve

Re: [HACKERS] Latch for the WAL writer - further reducing idle wake-ups.

2012-05-03 Thread Magnus Hagander
On Thu, May 3, 2012 at 2:41 AM, Robert Haas robertmh...@gmail.com wrote: On Wed, May 2, 2012 at 7:21 PM, Tom Lane t...@sss.pgh.pa.us wrote: It is getting a bit late to be considering such changes for 9.2, but I'm willing to review and commit this if there's not anybody who feels strongly that

Re: [HACKERS] Latch for the WAL writer - further reducing idle wake-ups.

2012-05-03 Thread Peter Geoghegan
On 3 May 2012 10:56, Magnus Hagander mag...@hagander.net wrote: I agree that it's ok to slip it in given that it's finishing off a patch from earlier. I think it's reasonable to hold it to a little bit higher review stadards since it's that late in the cycle though, such as two people

[HACKERS] Latch for the WAL writer - further reducing idle wake-ups.

2012-05-02 Thread Peter Geoghegan
Attached patch latches up the WAL Writer, reducing wake-ups and thus saving electricity in a way that is more-or-less analogous to my work on the BGWriter: http://git.postgresql.org/gitweb/?p=postgresql.git;a=commit;h=6d90eaaa89a007e0d365f49d6436f35d2392cfeb I am hoping this gets into 9.2 . I am

Re: [HACKERS] Latch for the WAL writer - further reducing idle wake-ups.

2012-05-02 Thread Tom Lane
Peter Geoghegan pe...@2ndquadrant.com writes: Attached patch latches up the WAL Writer, reducing wake-ups and thus saving electricity in a way that is more-or-less analogous to my work on the BGWriter:

Re: [HACKERS] Latch for the WAL writer - further reducing idle wake-ups.

2012-05-02 Thread Robert Haas
On Wed, May 2, 2012 at 7:21 PM, Tom Lane t...@sss.pgh.pa.us wrote: It is getting a bit late to be considering such changes for 9.2, but I'm willing to review and commit this if there's not anybody who feels strongly that it's too late.  Personally I think it's in the nature of cleanup and so

Re: [HACKERS] Latch for the WAL writer - further reducing idle wake-ups.

2012-05-02 Thread Tom Lane
Robert Haas robertmh...@gmail.com writes: ... It seems unlikely to cause any real problem if WAL writer takes a couple seconds to get with the program after a long period of inactivity; note that an async commit will kick it anyway, and a sync commit will probably half to flush WAL whether

Re: [HACKERS] Latch for the WAL writer - further reducing idle wake-ups.

2012-05-02 Thread Robert Haas
On Wed, May 2, 2012 at 11:42 PM, Tom Lane t...@sss.pgh.pa.us wrote: Robert Haas robertmh...@gmail.com writes: ... It seems unlikely to cause any real problem if WAL writer takes a couple seconds to get with the program after a long period of inactivity; note that an async commit will kick it