Re: [HACKERS] Detrimental performance impact of ringbuffers onperformance

Hi,

On 2019-05-08 10:08:03 -0400, Robert Haas wrote:
> On Tue, May 7, 2019 at 4:16 PM Andres Freund <andres@anarazel.de> wrote:
> > Just to attach some numbers for this. On my laptop, with a pretty fast
> > disk (as in ~550MB/s read + write, limited by SATA, not the disk), I get
> > these results.
> >
> >  [ results showing ring buffers massively hurting performance ]
>
> Links to some previous discussions:
>
> http://postgr.es/m/8737e9bddb82501da1134f021bf4929a@postgrespro.ru
> http://postgr.es/m/CAMkU=1yV=Zq8sHviv5Nwajv5woWOvZb7bx45rgDvtxs4P6W1Pw@mail.gmail.com
>
> > We probably can't remove the ringbuffer concept from these places, but I
> > think we should allow users to disable them. Forcing bulk-loads, vacuum,
> > analytics queries to go to the OS/disk, just because of a heuristic that
> > can't be disabled, yielding massive slowdowns, really sucks.
>
> The discussions to which I linked above seem to suggest that one of
> the big issues is that the ring buffer must be large enough that WAL
> flush for a buffer can complete before we go all the way around the
> ring and get back to the same buffer.

That is some of the problem, true. But even on unlogged tables the
ringbuffers cause quite the massive performance deterioration. Without
the ringbuffers we write twice the size of the releation (once with
zeroes for the file extension, once with with the actual data). With the
ringbuffer we do so two or three additional times (hint bits + normal
vacuum, then freezing).

On a test-cluster that replaced the smgrextend() for heap with
posix_fallocate() (to avoid the unnecesary write), I measured the
performance of CTAS UNLOGGED SELECT * FROM pgbench_accounts_scale_1000
with and without ringbuffers:

With ringbuffers:
CREATE UNLOGGED TABLE AS: Time: 67808.643 ms (01:07.809)
VACUUM: Time: 53020.848 ms (00:53.021)
VACUUM FREEZE: Time: 55809.247 ms (00:55.809)

Without ringbuffers:
CREATE UNLOGGED TABLE AS: Time: 45981.237 ms (00:45.981)
VACUUM: Time: 23386.818 ms (00:23.387)
VACUUM FREEZE: Time: 5892.204 ms (00:05.892)



> It doesn't seem unlikely that
> the size necessary for that to be true has changed over the years, or
> even that it's different on different hardware.  When I did some
> benchmarking in this area many years ago, I found that there as you
> increase the ring buffer size, performance improves for a while and
> then more or less levels off at a certain point.  And at that point
> performance is not much worse than it would be with no ring buffer,
> but you maintain some protection against cache-trashing.  Your
> scenario assumes that the system has no concurrent activity which will
> suffer as a result of blowing out the cache, but in general that's
> probably not true.

Well, I noted that I'm not proposing to actually just rip out the
ringbuffers.

But I also don't think it's just a question of concurrent activity. It's
a question of having concurrent activity *and* workloads that are
smaller than shared buffers.

Given current memory sizes a *lot* of workloads fit entirely in shared
buffers - but for vacuum, seqscans (including copy), it's basically
impossible to ever take advantage of that memory, unless your workload
otherwise forces it into s_b entirely (or you manually load the data
into s_b).


> It seems to me that it might be time to bite the bullet and add GUCs
> for the ring buffer sizes.  Then, we could make the default sizes big
> enough that on normal-ish hardware the performance penalty is not too
> severe (like, it's measured as a percentage rather than a multiple),
> and we could make a 0 value disable the ring buffer altogether.

Yea, it'd be considerably better than today. It'd importantly allow us
to more easily benchmark a lot of this.

Think it might make sense to have a VACUUM option for disabling the
ringbuffer too, especially for cases where VACUUMING is urgent.


I think what we ought to do to fix this issue in a bit more principled
manner (afterwards) is:

1) For ringbuffer'ed scans, if there are unused buffers, use them,
   instead of recycling a buffer from the ring. If so, replace the
   previous member of the ring with the previously unused one.  When
   doing so, just reduce the usagecount by one (unless already zero), so
   it readily can be replaced.

   I think we should do so even when the to-be-replaced ringbuffer entry
   is currently dirty. But even if we couldn't agree on that, it'd
   already be a significant improvement if we only did this for clean buffers.

   That'd fix a good chunk of the "my shared buffers is never actually
   used" type issues. I personally think it's indefensible that
   we don't do that today.

2) When a valid buffer in the ringbuffer is dirty when about to be
   replaced, instead of doing the FlushBuffer ourselves (and thus
   waiting for an XLogFlush in many cases), put into a separate
   ringbuffer/qeueue that's processed by bgwriter. And have that then
   invalidate the buffer and put it on the freelist (unless usagecount
   was bumped since, of course).

   That'd fix the issue that we're slowed down by constantly doing
   XLogFlush() for fairly small chunks of WAL.

3) When, for a ringbuffer scan, there are no unused buffers, but buffers
   with a zero-usagecount, use them too without evicting the previous
   ringbuffer entry. But do so without advancing the normal clock sweep
   (i.e. decrementing usagecounts). That allows to slowly replace buffer
   contents with data accessed during ringbuffer scans.


Regards,

Andres