On 2014-09-11 09:48:10 -0400, Robert Haas wrote:
> On Thu, Sep 11, 2014 at 9:22 AM, Andres Freund <andres@2ndquadrant.com> wrote:
> > I wonder if we should recheck the number of freelist items before
> > sleeping. As the latch currently is reset before sleeping (IIRC) we
> > might miss being woken up soon. It very well might be that bgreclaim
> > needs to run for more than one cycle in a row to keep up...
>
> The outer loop in BgMoveBuffersToFreelist() was added to address
> precisely this point, which I raised in a previous review.
Hm, right. But then let's move BgWriterStats.m_buf_alloc =+,
... pgstat_send_bgwriter(); into that loop. Otherwise it'd possibly end
up being continously busy without being visible.
> >> > I wonder if we don't want to increase the high watermark when
> >> > tmp_recent_backend_clocksweep > 0?
> >>
> >> That doesn't really work unless there's some countervailing force to
> >> eventually reduce it again; otherwise, it'd just converge to infinity.
> >> And it doesn't really seem necessary at the moment.
> >
> > Right, it obviously needs to go both ways. I'm a bit sceptic about
> > untunable, fixed, numbers proving to be accurate for widely varied
> > workloads.
>
> Me, too, but I'm *even more* skeptical about making things complicated
> on the pure theory that a simple solution can't be correct.
Fair enough.
> I'm not
> blind to the possibility that the current logic is inadequate, but
> testing proves that it works well enough to produce a massive
> performance boost over where we are now.
But, to be honest, the testing so far was pretty "narrow" in the kind of
workloads that were run if I crossread things accurately. Don't get me
wrong, I'm *really* happy about having this patch, that just doesn't
mean every detail is right ;)
> >> > Hm. Perhaps we should do a bufHdr->refcount != zero check without
> >> > locking here? The atomic op will transfer the cacheline exclusively to
> >> > the reclaimer's CPU. Even though it very shortly afterwards will be
> >> > touched afterwards by the pinning backend.
> >>
> >> Meh. I'm not in favor of adding more funny games with locking unless
> >> we can prove they're necessary for performance.
> >
> > Well, this in theory increases the number of processes touching buffer
> > headers regularly. Currently, if you have one read IO intensive backend,
> > there's pretty much only process touching the cachelines. This will make
> > it two. I don't think it's unreasonable to try to reduce the cacheline
> > pingpong caused by that...
>
> It's not unreasonable, but this is a good place to apply Knuth's first
> law of optimization. There's no proof we need to do this, so let's
> not until there is.
That's true for new (pieces of) software; less so, when working with a
installed base that you might regress... But whatever.
Greetings,
Andres Freund
-- Andres Freund http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training &
Services