Subject: Pre-allocating WAL files

From: Andres Freund <[email-masked]>
Date: Fri, 25 Dec 2020 12:09:53 -0800

Hi,

When running write heavy transactional workloads I've many times
observed that one needs to run the benchmarks for quite a while till
they get to their steady state performance. The most significant reason
for that is that initially WAL files will not get recycled, but need to
be freshly initialized. That's 16MB of writes that need to synchronously
finish before a small write transaction can even start to be written
out...

I think there's two useful things we could do:

1) Add pg_wal_preallocate(uint64 bytes) that ensures (bytes +
   segment_size - 1) / segment_size WAL segments exist from the current
   point in the WAL. Perhaps with the number of bytes defaulting to
   min_wal_size if not explicitly specified?

2) Have checkpointer (we want walwriter to run with low latency to flush
   out async commits etc) occasionally check if WAL files need to be
   pre-allocated.

   Checkpointer already tracks the amount of WAL that's expected to be
   generated till the end of the checkpoint, so it seems like it's a
   pretty good candidate to do so.

   To keep checkpointer pre-allocating when idle we could signal it
   whenever a record has crossed a segment boundary.

With a plain pgbench run I see a 2.5x reduction in throughput in the
periods where we initialize WAL files.

Greetings,

Andres Freund

---

From: "Bossart, Nathan" <[email-masked]>
Date: Mon, 7 Jun 2021 15:18:06 +0000

> periods where we initialize WAL files.

I've been exploring this independently a bit and noticed this message.
Attached is a proof-of-concept patch for a separate "WAL allocator"
process that maintains a pool of WAL-segment-sized files that can be
claimed whenever a new segment file is needed.  An early version of
this patch attempted to spread the I/O like non-immediate checkpoints
do, but I couldn't point to any real benefit from doing so, and it
complicated things quite a bit.

I like the idea of trying to bake this into an existing process such
as the checkpointer.  I'll admit that creating a new process just for
WAL pre-allocation feels a bit heavy-handed, but it was a nice way to
keep this stuff modularized.  I can look into moving this
functionality into the checkpointer process if this is something that
folks are interested in.

Nathan

---

From: vignesh C <[email-masked]>
Date: Mon, 5 Jul 2021 22:22:27 +0530

> functionality into the checkpointer process if this is something that

Thanks for posting the patch, the patch no more applies on Head:
Applying: wal segment pre-allocation
error: patch failed: src/backend/access/transam/xlog.c:3283
error: src/backend/access/transam/xlog.c: patch does not apply

Can you rebase the patch and post, it might help if someone is picking
it up for review.

Regards,
Vignesh

---

From: "Bossart, Nathan" <[email-masked]>
Date: Fri, 9 Jul 2021 21:08:27 +0000

> Can you rebase the patch and post, it might help if someone is picking

I've attached a rebased version of the patch.

Nathan

---

From: "Bossart, Nathan" <[email-masked]>
Date: Fri, 6 Aug 2021 20:25:26 +0000

> I've attached a rebased version of the patch.

Here's a newer rebased version of the patch.

Nathan

---

From: "Bossart, Nathan" <[email-masked]>
Date: Tue, 31 Aug 2021 17:24:49 +0000

> Here's a newer rebased version of the patch.

Rebasing again to keep http://commitfest.cputube.org/ happy.

Nathan

---

From: "Bossart, Nathan" <[email-masked]>
Date: Tue, 14 Sep 2021 20:06:29 +0000

> Rebasing again to keep http://commitfest.cputube.org/ happy.

Another rebase.

Nathan

---

From: Maxim Orlov <[email-masked]>
Date: Wed, 06 Oct 2021 12:19:23 +0000

The following review has been posted through the commitfest application:
make installcheck-world:  tested, passed
Implements feature:       tested, failed
Spec compliant:           not tested
Documentation:            not tested

Hi!

We've looked through the code and everything looks good except few minor things:
1). Using dedicated bg worker seems not optimal, it introduces a lot of redundant code for little single action.
    We'd join initial proposal of Andres to implement it as an extra fuction of checkpointer.
2). In our view, it is better shift #define PREALLOCSEGDIR outside the function body.
3). It is better to have at least small comments on functions GetNumPreallocatedWalSegs, SetNumPreallocatedWalSegs, 

We've also tested performance difference between master branch and this patch and noticed no significant difference in performance.
We used pgbench with some sort of "standard" settings:
$ pgbench -c50 -s50 -T200 -P1 -r postgres
...and with...
$ pgbench -c100 -s50 -T200 -P1 -r postgres

When looking at every second output of pgbench we saw regular spikes of latency (aroud 5-10 times increase) and this pattern was similar with and without patch. Overall average latency stat for 200 sec of pgbench also looks pretty much the same with and without patch. Could you provide your testing setup to see the effect, please.

The check-world was successfull.

Overall patch looks good, but in our view it's better to have experimental support of the performance improvements to be commited.

---
Best regards,
Maxim Orlov, Pavel Borisov.

The new status of this patch is: Waiting on Author

---

From: "Bossart, Nathan" <[email-masked]>
Date: Wed, 6 Oct 2021 16:31:42 +0000

>     We'd join initial proposal of Andres to implement it as an extra fuction of checkpointer.

Thanks for taking a look!

I have been thinking about the right place to put this logic.  My
first thought is that it sounds like something that ought to go in the
WAL writer process, but as Andres noted upthread, that is undesirable
because it'll add latency for asynchronous commits.  The checkpointer
process seems to be another candidate, but I'm not totally sure how
it'll fit in.  My patch works by maintaining a small pool of pre-
allocated segments that is quickly replenished whenever one is used.
If the checkpointer is spending most of its time checkpointing, this
small pool could remain empty for long periods of time.  To keep pre-
allocating WAL while we're checkpointing, perhaps we could add another
function like CheckpointWriteDelay() that is called periodically.
There still might be several operations in CheckPointGuts() that take
a while and leave the segment pool empty, but maybe that's okay for
now.

Nathan

---

From: "Bossart, Nathan" <[email-masked]>
Date: Fri, 8 Oct 2021 20:48:58 +0000

> a while and leave the segment pool empty, but maybe that's okay for

Here is a first attempt at adding the pre-allocation logic to the
checkpointer.  I went ahead and just used CheckpointWriteDelay() for
pre-allocating during checkpoints.  I've done a few pgbench runs, and
it seems to work pretty well.  Initialization is around 15% faster,
and I'm seeing about a 5% increase in TPS with a simple-update
workload with wal_recycle turned off.  Of course, these improvements
go away once segments can be recycled.

Nathan

---

From: "Bossart, Nathan" <[email-masked]>
Date: Wed, 10 Nov 2021 18:59:43 +0000

> go away once segments can be recycled.

Here is a rebased version of this patch set.  I'm getting the sense
that there isn't a whole lot of interest in this feature, so I'll
likely withdraw it if it goes too much longer without traction.

Nathan

---

From: Bharath Rupireddy <[email-masked]>
Date: Tue, 7 Dec 2021 13:57:56 +0530

> likely withdraw it if it goes too much longer without traction.

As I mentioned in the other thread at [1], let's continue the discussion here.

Why can't the walwriter pre-allocate some of the WAL segments instead
of a new background process? Of course, it might delay the main
functionality of the walwriter i.e. flush and sync the WAL files, but
having checkpointer do the pre-allocation makes it do another extra
task. Here the amount of walwriter work vs checkpointer work, I'm not
sure which one does more work compared to the other.

Another idea could be to let walwrtier or checkpointer pre-allocate
the WAL files whichever seems free as-of-the-moment when the WAL
segment pre-allocation request comes. We can go further to let the
user choose which process i.e. checkpointer or walwrtier do the
pre-allocation with a GUC maybe?

[1] - https://www.postgresql.org/message-id/CALj2ACVqYJX9JugooRC1chb2sHqv-C9mYEBE1kxwn%2BTn9vY42A%40mail.gmail.com

Regards,
Bharath Rupireddy.

---

From: "Bossart, Nathan" <[email-masked]>
Date: Tue, 7 Dec 2021 17:34:46 +0000

> sure which one does more work compared to the other.

The argument against adding it to the WAL writer is that we want it to
run with low latency to flush asynchronous commits.  If we added WAL
pre-allocation to the WAL writer, there could periodically be large
delays.

> pre-allocation with a GUC maybe?

My latest patch set [0] adds WAL pre-allocation to the checkpointer.
In that patch set, WAL pre-allocation is done both outside of
checkpoints as well as during checkpoints (via
CheckPointWriteDelay()).  

Nathan

[0] https://www.postgresql.org/message-id/CB15BEBD-98FC-4E72-86AE-513D34014176%40amazon.com

---

From: "Bossart, Nathan" <[email-masked]>
Date: Tue, 7 Dec 2021 21:01:12 +0000

> pre-allocation to the WAL writer, there could periodically be large

To your point on trying to avoid giving the checkpointer extra tasks
(basically what we are talking about on the other thread [0]), WAL
pre-allocation might not be of much concern because it will generally
be a small, fixed (and configurable) amount of work, and it can be
performed concurrently with the checkpoint.  Plus, WAL pre-allocation
should ordinarily be phased out as WAL segments become eligible for
recycling.  IMO it's not comparable to tasks like
CheckPointSnapBuild() that can delay checkpointing for a long time.

Nathan

[0] https://www.postgresql.org/message-id/flat/C1EE64B0-D4DB-40F3-98C8-0CED324D34CB%40amazon.com

---

From: Pavel Borisov <[email-masked]>
Date: Thu, 30 Dec 2021 15:45:42 +0400

> > go away once segments can be recycled.

I've checked the patch v7. It applies cleanly, code is good, check-world
tests passed without problems.
I think it's ok to use checkpointer for this and the overall patch can be
committed. But the seen performance gain makes me think again before adding
this feature. I did tests myself a couple of months ago and got similar
results.
Really don't know whether is it worth the effort.

Wish you and all hackers happy New Year!
-- 
Best regards,
Pavel Borisov

Postgres Professional: http://postgrespro.com <http://www.postgrespro.com>

---

From: Maxim Orlov <[email-masked]>
Date: Thu, 30 Dec 2021 14:51:10 +0300

I did check the patch too and found it to be ok. Check and check-world are
passed.
Overall idea seems to be good in my opinion, but I'm not sure where is the
optimal place to put the pre-allocation.

-- 
---
Best regards,
Maxim Orlov.

---

From: Nathan Bossart <[email-masked]>
Date: Thu, 14 Jul 2022 11:34:07 -0700

On Fri, Apr 08, 2022 at 01:30:03PM -0700, Nathan Bossart wrote:
> On Thu, Mar 17, 2022 at 04:12:12PM -0700, Nathan Bossart wrote:
>> It seems unlikely that this will be committed for v15, so I've adjusted the
>> commitfest entry to v16 and moved it to the next commitfest.
> 
> rebased

It's now been over a year since I first posted a patch in this thread, and
I still sense very little interest for this feature.  I intend to mark it
as Withdrawn at the end of this commitfest.

-- 
Nathan Bossart
Amazon Web Services: https://aws.amazon.com

---

From: Nathan Bossart <[email-masked]>
Date: Mon, 25 Jul 2022 09:24:17 -0700

> as Withdrawn at the end of this commitfest.

Done.