From: | Maxim Orlov <m(dot)orlov(at)postgrespro(dot)ru> |
---|---|
To: | pgsql-hackers(at)lists(dot)postgresql(dot)org |
Cc: | Nathan Bossart <bossartn(at)amazon(dot)com> |
Subject: | Re: Pre-allocating WAL files |
Date: | 2021-10-06 12:19:23 |
Message-ID: | 163352276350.15435.3988688227048304232.pgcf@coridan.postgresql.org |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
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 | Date | Subject | |
---|---|---|---|
Next Message | Amul Sul | 2021-10-06 13:19:10 | Re: TAP test for recovery_end_command |
Previous Message | Juan José Santamaría Flecha | 2021-10-06 12:17:34 | Re: Capitalization of localized month and day names (to_char() with 'TMmonth', 'TMday', etc.) |