On 2/4/17 2:47 PM, Alexander Korotkov wrote: > On Sat, Feb 4, 2017 at 4:33 AM, Andres Freund <and...@anarazel.de > <mailto:and...@anarazel.de>> wrote: > > On 2017-02-03 19:13:45 -0600, Jim Nasby wrote: > > No, I noticed it while reading code. Removing that does mean that if any > > non-default strategy (in any backend) hits that buffer again then the > buffer > > will almost certainly migrate into the main buffer pool the next time > one of > > the rings hits that buffer > > Well, as long as the buffer is used from the ring, BufferAlloc() - > BufferAlloc() will reset the usagecount when rechristening the > buffer. So unless anything happens inbetween the buffer being remapped > last and remapped next, it'll be reused. Right? > > The only case where I can see the old logic mattering positively is for > synchronized seqscans. For pretty much else that logic seems worse, > because it essentially prevents any buffers ever staying in s_b when > only ringbuffer accesses are performed. > > I'm tempted to put the old logic back, but more because this likely was > unintentional, not because I think it's clearly better. > > > +1 > Yes, it was unintentional change. So we should put old logic back > unless we have proof that this change make it better. > Patch is attached. I tried to make some comments, but probably they are > not enough.
This patch looks pretty straight forward and applies cleanly and compiles at cccbdde. It's not a straight revert, though, so still seems to need review. Jim, do you know when you'll have a chance to look at that? -- -David da...@pgmasters.net -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers