Re: [HACKERS] Increase Vacuum ring buffer.

On 2017-07-20 17:59, Robert Haas wrote:
> On Tue, Jul 18, 2017 at 6:09 AM, Sokolov Yura
> <funny.falcon@postgrespro.ru> wrote:
>> I investigated autovacuum performance, and found that it suffers a lot
>> from small ring buffer. It suffers in a same way bulk writer suffered
>> before Tom Lane's commit 6382448cf96:
>> 
>>> Tom Lane <tgl@sss.pgh.pa.us>  2009-06-23 00:04:28
>>> For bulk write operations (eg COPY IN), use a ring buffer of 16MB
>>> instead of the 256KB limit originally enforced by a patch committed
>>> 2008-11-06. Per recent test results, the smaller size resulted in an
>>> undesirable decrease in bulk data loading speed, due to COPY
>>> processing frequently getting blocked for WAL flushing. This area
>>> might need more tweaking later, but this setting seems to be good
>>> enough for 8.4.
>> 
>> It is especially noticable when database doesn't fit in shared_buffers
>> but fit into OS file cache, and data is intensively updated (ie OLTP
>> load). In this scenario autovacuum with current 256kB (32 pages) ring
>> buffer lasts 3-10 times longer than with increased to 16MB ring 
>> buffer.
>> 
>> I've tested with synthetic load with 256MB or 1GB shared buffers and
>> 2-6GB (with indices) tables, with different load factor and 
>> with/without
>> secondary indices on updated columns. Table were randomly updated with
>> hot and non-hot updates. Times before/after buffer increase (depending
>> on load) were 7500sec/1200sec, 75000sec/11500sec. So benefit is
>> consistently reproducible.
>> 
>> I didn't tested cases when database doesn't fit OS file cache. 
>> Probably
>> in this case benefit will be smaller cause more time will be spent in
>> disk read.
>> I didn't test intensively OLAP load. I've seen once that increased
>> buffer slows a bit scanning almost immutable huge table, perhaps cause
>> of decreased CPU cache locality. But given such scan is already fast,
>> and autovacuum of "almost immutable table" runs rarely, I don't think
>> it is very important.
>> 
>> Initially I wanted to make BAS_BULKWRITE and BAS_VACUUM ring sizes
>> configurable, but after testing I don't see much gain from increasing
>> ring buffer above 16MB. So I propose just 1 line change.
> 
> I think the question for this patch is "so, why didn't we do it this
> way originally?".
> 
> It's no secret that making the ring buffer larger will improve
> performance -- in fact, not having a ring buffer at all would improve
> performance even more.  But it would also increase the likelihood that
> the background work of vacuum would impact the performance of
> foreground operations, which is already a pretty serious problem that
> we probably don't want to make worse.  I'm not certain what the right
> decision is here, but I think that a careful analysis of possible
> downsides is needed.
> 
> --
> Robert Haas
> EnterpriseDB: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company

Initially, ring buffer were introduced for sequential scan.
It was added for vacuum "for a company", and before introducing
vacuum used just 1 page, so giving 32 pages to was huge improvement:

d526575f893c1a4e05ebd Tom Lane <tgl@sss.pgh.pa.us>  2007-05-31 00:12:03
"Make large sequential scans and VACUUMs work in a limited-size "ring" 
of buffers, rather than blowing out the whole shared-buffer arena.  Aside 
from avoiding cache spoliation, this fixes the problem that VACUUM formerly 
tended to cause a WAL flush for every page it modified, because we had it 
hacked to use only a single buffer."

Later ring buffer were added for bulk writer, but the same 32 pages:

85e2cedf985bfecaf43a18ca Tom Lane <tgl@sss.pgh.pa.us>  2008-11-06 
23:51:15
" Improve bulk-insert performance by keeping the current target buffer 
pinned  (but not locked, as that would risk deadlocks).  Also, make it work in 
a small  ring of buffers to avoid having bulk inserts trash the whole buffer 
arena.  Robert Haas, after an idea of Simon Riggs'."

And finally after some real world usage buffer for bulk writer were 
increased

6382448cf96a9b88 Tom Lane <tgl@sss.pgh.pa.us>  2009-06-23 00:04:28
" For bulk write operations (eg COPY IN), use a ring buffer of 16MB 
instead  of the 256KB limit originally enforced by a patch committed 
2008-11-06.  Per recent test results, the smaller size resulted in an undesirable 
decrease  in bulk data loading speed, due to COPY processing frequently getting 
blocked  for WAL flushing.  This area might need more tweaking later, but this 
setting  seems to be good enough for 8.4."

So, from my point of view, no one just evaluate performance of increased
ring buffer for vacuum.

It were discussed year age: 

https://www.postgresql.org/message-id/flat/CA%2BTgmobmP%3DKE-z5f7-CegXMFGRbV%3DhC%2B%3DFxb2mbhpfD-ZD%3D-bA%40mail.gmail.com#CA+TgmobmP=KE-z5f7-CegXMFGRbV=hC+=Fxb2mbhpfD-ZD=-bA@mail.gmail.com

There was your, Robert wrong assumption:
> But all that does is force the backend to write to the operating
system, which is where the real buffering happens.

But in fact, vacuum process performs FSYNC! It happens, cause vacuum
evicts dirty pages from its ring buffer. And to evict dirty page, it
has to be sure WAL record about its modification is FSYNC-ed to WAL.
Because ring buffer is damn small, vacuum almost always have to perform
FSYNC by itself and have to do it very frequently (cause ring is damn
small).

With greater ring buffer, there is greater chance that fsync were
performed by wal_writer process, or other backend. And even if
vacuum does fsync by itself, it syncs records about more modified
pages from its ring, so evicting next page is free.

Also it were discussed in 2012-2013 

https://www.postgresql.org/message-id/flat/CA%2BTgmoZ3BOips7ot0tnSPO0yhKB3RUShDFoiYruoYXZDPr%3DptQ%40mail.gmail.com#CA+TgmoZ3BOips7ot0tnSPO0yhKB3RUShDFoiYruoYXZDPr=ptQ@mail.gmail.com

No decision were made, unfortunately.

If some fears that increasing vacuum ring buffer will lead to
decreasing transaction performance, then why it were not exhaustively
tested?

I had no evidence that transactions suffers from autovacuum improved
in this way. Perhaps I tested not precisely.

People, lets collect more results! Please test this one line change
with load of your choice and share the results!

If improving autovacuum doesn't hurt performance much, then why we
should live with databases bloated?

If such improvement really hurts performance, then it should be
documented by letters in pgsql-hackers, and comment should be put
into src/backend/storage/buffer/freelist.c .

And possible community will decide, that it should be GUC variable:
- if one prefers to keep its database unbloated, he could increase
vacuum ring buffer,
- otherwise just left it in "safe-but-slow" default.

With regards,
-- 
Sokolov Yura aka funny_falcon
Postgres Professional: https://postgrespro.ru
The Russian Postgres Company