public inbox for [email protected]
help / color / mirror / Atom feedFrom: Maxim Orlov <[email protected]>
To: [email protected]
Cc: Nathan Bossart <[email protected]>
Subject: Re: Pre-allocating WAL files
Date: Wed, 06 Oct 2021 12:19:23 +0000
Message-ID: <163352276350.15435.3988688227048304232.pgcf@coridan.postgresql.org> (raw)
In-Reply-To: <[email protected]>
References: <[email protected]>
<[email protected]>
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
view thread (19+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected]
Subject: Re: Pre-allocating WAL files
In-Reply-To: <163352276350.15435.3988688227048304232.pgcf@coridan.postgresql.org>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox